Merge pull request #1116 from finos/fix-computed-charts

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

azure-pipelines.yml

@@ -369,7 +369,7 @@ jobs:
     - script: npm install -g yarn
       displayName: "Install Yarn"
 
-    - script: yarn
+    - script: yarn --network-timeout 600000
       displayName: 'Install Deps'
 
     - script: yarn build_python --ci $(python_flag)

cpp/perspective/src/cpp/aggspec.cpp

@@ -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";
@@ -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:
@@ -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: {

cpp/perspective/src/cpp/base.cpp

@@ -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") {

cpp/perspective/src/cpp/computed_function.cpp

@@ -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;
 }
@@ -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;
 }
@@ -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;
 }
@@ -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;
 }

cpp/perspective/src/cpp/config.cpp

@@ -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:

cpp/perspective/src/cpp/extract_aggregate.cpp

@@ -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:

cpp/perspective/src/cpp/sparse_tree.cpp

@@ -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);

cpp/perspective/src/cpp/view_config.cpp

@@ -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 {

cpp/perspective/src/include/perspective/base.h

@@ -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,

packages/perspective-viewer-d3fc/src/js/charts/treemap.js

@@ -48,6 +48,8 @@ function treemap(container, settings) {
         .each(function({split, data}) {
             const treemapSvg = d3.select(this);
 
+            console.log(split, data);
+
             if (!settings.treemaps[split]) settings.treemaps[split] = {};
 
             treemapSeries()

packages/perspective-viewer-d3fc/src/js/plugin/plugin.js

@@ -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
         };

packages/perspective-viewer-d3fc/test/js/integration/line.spec.js

@@ -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, "..", "..", "..")}
     );

packages/perspective-viewer-d3fc/test/results/linux.docker.json

@@ -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",
@@ -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"
 }

packages/perspective-viewer-highcharts/src/js/highcharts/draw.js

@@ -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 {
@@ -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;
@@ -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];

packages/perspective-viewer-highcharts/test/js/axis_tests.js

@@ -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])");
+        });
     });
 };