Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly display computed axis on charts, use local time for all datetime functions #1116

Merged
merged 11 commits into from
Jul 26, 2020
2 changes: 1 addition & 1 deletion azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ jobs:
- script: npm install -g yarn
displayName: "Install Yarn"

- script: yarn
- script: yarn --network-timeout 600000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

displayName: 'Install Deps'

- script: yarn build_python --ci $(python_flag)
Expand Down
7 changes: 3 additions & 4 deletions cpp/perspective/src/cpp/aggspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ t_aggspec::agg_str() const {
case AGGTYPE_FIRST: {
return "first";
} break;
case AGGTYPE_LAST: {
return "last";
case AGGTYPE_LAST_BY_INDEX: {
return "last_by_index";
} break;
case AGGTYPE_PY_AGG: {
return "py_agg";
Expand Down Expand Up @@ -297,7 +297,7 @@ t_aggspec::get_output_specs(const t_schema& schema) const {
case AGGTYPE_DOMINANT:
case AGGTYPE_MEDIAN:
case AGGTYPE_FIRST:
case AGGTYPE_LAST:
case AGGTYPE_LAST_BY_INDEX:
case AGGTYPE_OR:
case AGGTYPE_LAST_VALUE:
case AGGTYPE_HIGH_WATER_MARK:
Expand All @@ -318,7 +318,6 @@ t_aggspec::get_output_specs(const t_schema& schema) const {
return mk_col_name_type_vec(name(), DTYPE_F64PAIR);
}
case AGGTYPE_WEIGHTED_MEAN: {

return mk_col_name_type_vec(name(), DTYPE_F64PAIR);
}
case AGGTYPE_JOIN: {
Expand Down
2 changes: 1 addition & 1 deletion cpp/perspective/src/cpp/base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ str_to_aggtype(const std::string& str) {
} else if (str == "first by index" || str == "first") {
return t_aggtype::AGGTYPE_FIRST;
} else if (str == "last by index") {
return t_aggtype::AGGTYPE_LAST;
return t_aggtype::AGGTYPE_LAST_BY_INDEX;
} else if (str == "py_agg") {
return t_aggtype::AGGTYPE_PY_AGG;
} else if (str == "and") {
Expand Down
77 changes: 44 additions & 33 deletions cpp/perspective/src/cpp/computed_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -617,18 +617,20 @@ t_tscalar day_bucket<DTYPE_TIME>(t_tscalar x) {
// Convert the timestamp to a `sys_time` (alias for `time_point`)
date::sys_time<std::chrono::milliseconds> ts(ms_timestamp);

// Create a copy of the timestamp with day precision
auto days = date::floor<date::days>(ts);
// Use localtime so that the day of week is consistent with all output
// datetimes, which are in local time
std::time_t temp = std::chrono::system_clock::to_time_t(ts);

// Cast the `time_point` to contain year/month/day
auto ymd = date::year_month_day(days);
// Convert to a std::tm
std::tm* t = std::localtime(&temp);

// Get the year and create a new `t_date`
std::int32_t year = static_cast<std::int32_t>(ymd.year());
std::int32_t year = static_cast<std::int32_t>(t->tm_year + 1900);

// Month in `t_date` is [0-11]
std::int32_t month = static_cast<std::uint32_t>(ymd.month()) - 1;
std::uint32_t day = static_cast<std::uint32_t>(ymd.day());
std::int32_t month = static_cast<std::uint32_t>(t->tm_mon);
std::uint32_t day = static_cast<std::uint32_t>(t->tm_mday);

rval.set(t_date(year, month, day));
return rval;
}
Expand Down Expand Up @@ -679,21 +681,34 @@ t_tscalar week_bucket<DTYPE_TIME>(t_tscalar x) {
// Convert the timestamp to a `sys_time` (alias for `time_point`)
date::sys_time<std::chrono::milliseconds> ts(timestamp);

// Create a copy of the timestamp with day precision
auto days = date::floor<date::days>(ts);
// Convert the timestamp to local time
std::time_t temp = std::chrono::system_clock::to_time_t(ts);
std::tm* t = std::localtime(&temp);

// Take the ymd from the `tm`, now in local time, and create a
// date::year_month_day.
date::year year {1900 + t->tm_year};

// date::month is [1-12], whereas `std::tm::tm_mon` is [0-11]
date::month month {static_cast<std::uint32_t>(t->tm_mon) + 1};
date::day day {static_cast<std::uint32_t>(t->tm_mday)};
date::year_month_day ymd(year, month, day);

// Convert to a `sys_days` representing no. of days since epoch
date::sys_days days_since_epoch = ymd;

// Subtract Sunday from the ymd to get the beginning of the last day
date::year_month_day ymd = days - (date::weekday{days} - date::Monday);
ymd = days_since_epoch - (date::weekday{days_since_epoch} - date::Monday);

// Get the day of month and day of the week
std::int32_t year = static_cast<std::int32_t>(ymd.year());
std::int32_t year_int = static_cast<std::int32_t>(ymd.year());

// date::month is [1-12], whereas `t_date.month()` is [0-11]
std::uint32_t month = static_cast<std::uint32_t>(ymd.month()) - 1;
std::uint32_t day = static_cast<std::uint32_t>(ymd.day());
std::uint32_t month_int = static_cast<std::uint32_t>(ymd.month()) - 1;
std::uint32_t day_int = static_cast<std::uint32_t>(ymd.day());

// Return the new `t_date`
t_date new_date = t_date(year, month, day);
t_date new_date = t_date(year_int, month_int, day_int);
rval.set(new_date);
return rval;
}
Expand All @@ -713,20 +728,18 @@ t_tscalar month_bucket<DTYPE_TIME>(t_tscalar x) {
if (x.is_none() || !x.is_valid()) return rval;

// Convert the int64 to a milliseconds duration timestamp
std::chrono::milliseconds timestamp(x.to_int64());
std::chrono::milliseconds ms_timestamp(x.to_int64());

// Convert the timestamp to a `sys_time` (alias for `time_point`)
date::sys_time<std::chrono::milliseconds> ts(timestamp);

// Create a copy of the timestamp with day precision
auto days = date::floor<date::days>(ts);
date::sys_time<std::chrono::milliseconds> ts(ms_timestamp);

// Cast the `time_point` to contain year/month/day
auto ymd = date::year_month_day(days);
// Convert the timestamp to local time
std::time_t temp = std::chrono::system_clock::to_time_t(ts);
std::tm* t = std::localtime(&temp);

// Get the year and create a new `t_date`
std::int32_t year = static_cast<std::int32_t>(ymd.year());
std::int32_t month = static_cast<std::uint32_t>(ymd.month()) - 1;
// Use the `tm` to create the `t_date`
std::int32_t year = static_cast<std::int32_t>(t->tm_year + 1900);
std::int32_t month = static_cast<std::uint32_t>(t->tm_mon);
rval.set(t_date(year, month, 1));
return rval;
}
Expand All @@ -747,19 +760,17 @@ t_tscalar year_bucket<DTYPE_TIME>(t_tscalar x) {
if (x.is_none() || !x.is_valid()) return rval;

// Convert the int64 to a milliseconds duration timestamp
std::chrono::milliseconds timestamp(x.to_int64());
std::chrono::milliseconds ms_timestamp(x.to_int64());

// Convert the timestamp to a `sys_time` (alias for `time_point`)
date::sys_time<std::chrono::milliseconds> ts(timestamp);

// Create a copy of the timestamp with day precision
auto days = date::floor<date::days>(ts);
date::sys_time<std::chrono::milliseconds> ts(ms_timestamp);

// Cast the `time_point` to contain year/month/day
auto ymd = date::year_month_day(days);
// Convert the timestamp to local time
std::time_t temp = std::chrono::system_clock::to_time_t(ts);
std::tm* t = std::localtime(&temp);

// Get the year and create a new `t_date`
std::int32_t year = static_cast<std::int32_t>(ymd.year());
// Use the `tm` to create the `t_date`
std::int32_t year = static_cast<std::int32_t>(t->tm_year + 1900);
rval.set(t_date(year, 0, 1));
return rval;
}
Expand Down
2 changes: 1 addition & 1 deletion cpp/perspective/src/cpp/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ t_config::setup(const std::vector<std::string>& detail_columns,
case AGGTYPE_OR:
case AGGTYPE_ANY:
case AGGTYPE_FIRST:
case AGGTYPE_LAST:
case AGGTYPE_LAST_BY_INDEX:
case AGGTYPE_MEAN:
case AGGTYPE_WEIGHTED_MEAN:
case AGGTYPE_UNIQUE:
Expand Down
2 changes: 1 addition & 1 deletion cpp/perspective/src/cpp/extract_aggregate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ extract_aggregate(
case AGGTYPE_DOMINANT:
case AGGTYPE_MEDIAN:
case AGGTYPE_FIRST:
case AGGTYPE_LAST:
case AGGTYPE_AND:
case AGGTYPE_OR:
case AGGTYPE_LAST_BY_INDEX:
case AGGTYPE_LAST_VALUE:
case AGGTYPE_HIGH_WATER_MARK:
case AGGTYPE_LOW_WATER_MARK:
Expand Down
2 changes: 1 addition & 1 deletion cpp/perspective/src/cpp/sparse_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ t_stree::update_agg_table(t_uindex nidx, t_agg_update_info& info, t_uindex src_r
dst->set_scalar(dst_ridx, new_value);
} break;
case AGGTYPE_FIRST:
case AGGTYPE_LAST: {
case AGGTYPE_LAST_BY_INDEX: {
old_value.set(dst->get_scalar(dst_ridx));
new_value.set(first_last_helper(nidx, spec, gstate));
dst->set_scalar(dst_ridx, new_value);
Expand Down
4 changes: 2 additions & 2 deletions cpp/perspective/src/cpp/view_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ t_view_config::fill_aggspecs(std::shared_ptr<t_schema> schema) {
}
}

if (agg_type == AGGTYPE_FIRST || agg_type == AGGTYPE_LAST) {
dependencies.push_back(t_dep("psp_pkey", DEPTYPE_COLUMN));
if (agg_type == AGGTYPE_FIRST || agg_type == AGGTYPE_LAST_BY_INDEX) {
dependencies.push_back(t_dep("psp_okey", DEPTYPE_COLUMN));
m_aggspecs.push_back(
t_aggspec(column, column, agg_type, dependencies, SORTTYPE_ASCENDING));
} else {
Expand Down
2 changes: 1 addition & 1 deletion cpp/perspective/src/include/perspective/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ enum t_aggtype {
AGGTYPE_SCALED_MUL,
AGGTYPE_DOMINANT,
AGGTYPE_FIRST,
AGGTYPE_LAST,
AGGTYPE_LAST_BY_INDEX,
AGGTYPE_PY_AGG,
AGGTYPE_AND,
AGGTYPE_OR,
Expand Down
2 changes: 2 additions & 0 deletions packages/perspective-viewer-d3fc/src/js/charts/treemap.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ function treemap(container, settings) {
.each(function({split, data}) {
const treemapSvg = d3.select(this);

console.log(split, data);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug leak


if (!settings.treemaps[split]) settings.treemaps[split] = {};

treemapSeries()
Expand Down
21 changes: 17 additions & 4 deletions packages/perspective-viewer-d3fc/src/js/plugin/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,35 @@ function drawChart(chart) {
jsonp = view.to_json({leaves_only: true});
}

let [tschema, schema, json, config] = await Promise.all([this._table.schema(false), view.schema(false), jsonp, view.get_config()]);
let [table_schema, computed_schema, view_schema, json, config] = await Promise.all([this._table.schema(false), view.computed_schema(false), view.schema(false), jsonp, view.get_config()]);

if (task.cancelled) {
return;
}

/**
* Retrieve a tree axis column from the table and computed schemas,
* returning a String type or `undefined`.
* @param {String} column a column name
*/
const get_pivot_column_type = function(column) {
let type = table_schema[column];
if (!type) {
type = computed_schema[column];
}
return type;
};

const {columns, row_pivots, column_pivots, filter} = config;
const filtered = row_pivots.length > 0 ? json.filter(col => col.__ROW_PATH__ && col.__ROW_PATH__.length == row_pivots.length) : json;
const dataMap = (col, i) => (!row_pivots.length ? {...col, __ROW_PATH__: [i]} : col);
const mapped = filtered.map(dataMap);

let settings = {
realValues,
crossValues: row_pivots.map(r => ({name: r, type: tschema[r]})),
mainValues: columns.map(a => ({name: a, type: schema[a]})),
splitValues: column_pivots.map(r => ({name: r, type: tschema[r]})),
crossValues: row_pivots.map(r => ({name: r, type: get_pivot_column_type(r)})),
mainValues: columns.map(a => ({name: a, type: view_schema[a]})),
splitValues: column_pivots.map(r => ({name: r, type: get_pivot_column_type(r)})),
filter,
data: mapped
};
Expand Down
13 changes: 13 additions & 0 deletions packages/perspective-viewer-d3fc/test/js/integration/line.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ utils.with_server({}, () => {
"line.html",
() => {
simple_tests.default();

test.capture("Sets a category axis when pivoted by a computed datetime", async page => {
const viewer = await page.$("perspective-viewer");
await page.shadow_click("perspective-viewer", "#config_button");
await page.evaluate(element => element.setAttribute("computed-columns", JSON.stringify(["hour_bucket('Ship Date')"])), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
await page.evaluate(element => element.setAttribute("row-pivots", '["hour_bucket(Ship Date)"]'), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
await page.evaluate(element => element.setAttribute("columns", '["State","Sales"]'), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
await page.evaluate(element => element.setAttribute("aggregates", '{"State":"dominant","Sales":"sum"}'), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
});
},
{reload_page: false, root: path.join(__dirname, "..", "..", "..")}
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"candlestick_filter_by_a_single_instrument_": "f988ca6494d7a36bada09928cd1a544e",
"candlestick_filter_to_date_range_": "8ca4da0a6229d4f9db4a845d5d415c20",
"__GIT_COMMIT__": "410648f3426fe2358cdf2a9a89424ed6cbe6122a",
"__GIT_COMMIT__": "66c751c99ac69898969049e164e63bb368e877dd",
"ohlc_filter_by_a_single_instrument_": "0110fac1f2befac1b97a9d33f0022acf",
"ohlc_filter_to_date_range_": "3ec466996be47e2c8df135a4303bf383",
"scatter_shows_a_grid_without_any_settings_applied_": "8677946ab48f16a376c421500d59e6c0",
Expand Down Expand Up @@ -125,5 +125,6 @@
"treemap_sorts_by_an_alpha_column_": "046d040908811f04c15f7646d9d70733",
"treemap_displays_visible_columns_": "62996aa87b1237b0be568a339a700bdf",
"treemap_with_column_position_1_set_to_null_": "a2f594ff864fb673cde708e4533d8edc",
"treemap_tooltip_columns_works": "9b3a44cc1b4b1d11cb8b635c850a0612"
"treemap_tooltip_columns_works": "9b3a44cc1b4b1d11cb8b635c850a0612",
"line_Sets_a_category_axis_when_pivoted_by_a_computed_datetime": "eb1c86dc44988ad9a65fdd5a335850b8"
}
29 changes: 21 additions & 8 deletions packages/perspective-viewer-highcharts/src/js/highcharts/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,22 +53,35 @@ export const draw = (mode, set_config, restyle) =>
const columns = config.columns;
const real_columns = JSON.parse(this.getAttribute("columns"));

const [schema, tschema] = await Promise.all([view.schema(false), this._table.schema(false)]);
const [view_schema, computed_schema, tschema] = await Promise.all([view.schema(false), view.computed_schema(false), this._table.schema(false)]);
let element;

if (task.cancelled) {
return;
}

/**
* Retrieve a tree axis column from the table and computed schemas,
* returning a String type or `undefined`.
* @param {String} column a column name
*/
const get_tree_type = function(column) {
let type = tschema[column];
if (!type) {
type = computed_schema[column];
}
return type;
};

let configs = [],
xaxis_name = columns.length > 0 ? columns[0] : undefined,
xaxis_type = schema[xaxis_name],
xaxis_type = view_schema[xaxis_name],
yaxis_name = columns.length > 1 ? columns[1] : undefined,
yaxis_type = schema[yaxis_name],
yaxis_type = view_schema[yaxis_name],
xtree_name = row_pivots.length > 0 ? row_pivots[row_pivots.length - 1] : undefined,
xtree_type = tschema[xtree_name],
xtree_type = get_tree_type(xtree_name),
ytree_name = col_pivots.length > 0 ? col_pivots[col_pivots.length - 1] : undefined,
ytree_type = tschema[ytree_name],
ytree_type = get_tree_type(ytree_name),
num_aggregates = columns.length;

try {
Expand All @@ -80,7 +93,7 @@ export const draw = (mode, set_config, restyle) =>
cols = await view.to_columns();
}
const config = (configs[0] = default_config.call(this, real_columns, mode));
const [series, xtop, colorRange, ytop] = make_xy_column_data(cols, schema, real_columns, row_pivots, col_pivots);
const [series, xtop, colorRange, ytop] = make_xy_column_data(cols, view_schema, real_columns, row_pivots, col_pivots);

config.legend.floating = series.length <= 20;
config.legend.enabled = col_pivots.length > 0;
Expand Down Expand Up @@ -158,15 +171,15 @@ export const draw = (mode, set_config, restyle) =>
} else {
cols = await view.to_columns();
}
s = await make_xy_column_data(cols, schema, columns, row_pivots, col_pivots);
s = await make_xy_column_data(cols, view_schema, columns, row_pivots, col_pivots);
} else {
let js;
if (end_col || end_row) {
js = await view.to_json({end_col, end_row, leaves_only: false});
} else {
js = await view.to_json();
}
s = await make_xy_data(js, schema, columns, row_pivots, col_pivots);
s = await make_xy_data(js, view_schema, columns, row_pivots, col_pivots);
}

const series = s[0];
Expand Down
11 changes: 11 additions & 0 deletions packages/perspective-viewer-highcharts/test/js/axis_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,16 @@ exports.default = function() {
await page.evaluate(element => element.setAttribute("columns", '["State","Sales"]'), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
});

test.capture("Sets a category axis when category is a computed datetime", async page => {
const viewer = await page.$("perspective-viewer");
await page.shadow_click("perspective-viewer", "#config_button");
await page.evaluate(element => element.setAttribute("computed-columns", JSON.stringify(["hour_bucket('Ship Date')"])), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
await page.evaluate(element => element.setAttribute("columns", '["hour_bucket(Ship Date)","Sales"]'), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
await page.evaluate(element => element.setAttribute("aggregates", '{"State":"dominant","Sales":"sum"}'), viewer);
await page.waitForSelector("perspective-viewer:not([updating])");
});
});
};
Loading