Merge pull request #1074 from finos/datetime-fixes

Use local time for column/row headers and computed functions

cpp/perspective/CMakeLists.txt

@@ -45,21 +45,26 @@ function (psp_build_dep name cmake_file)
 		message(WARNING "${Cyan}Dependency found - not rebuilding - ${CMAKE_BINARY_DIR}/${name}-build${ColorReset}")
 	else()
 		configure_file(${cmake_file} ${name}-download/CMakeLists.txt)
+
 		execute_process(COMMAND ${CMAKE_COMMAND} -G "${CMAKE_GENERATOR}" .
 			RESULT_VARIABLE result
 			WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/${name}-download )
+
 		if(result)
 			message(FATAL_ERROR "CMake step for ${name} failed: ${result}")
 		endif()
+
 		execute_process(COMMAND ${CMAKE_COMMAND} --build .
 			RESULT_VARIABLE result
 			WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/${name}-download )
+
 		if(result)
 			message(FATAL_ERROR "Build step for ${name} failed: ${result}")
 		endif()
 	endif()
 
 	if(${name} STREQUAL arrow)
+		# Overwrite arrow's CMakeLists with our custom, minimal one
 		configure_file(${PSP_CMAKE_MODULE_PATH}/arrow/CMakeLists.txt ${CMAKE_BINARY_DIR}/arrow-src/cpp/ COPYONLY)
 		configure_file(${PSP_CMAKE_MODULE_PATH}/arrow/config.h ${CMAKE_BINARY_DIR}/arrow-src/cpp/src/arrow/util/ COPYONLY)
 		add_subdirectory(${CMAKE_BINARY_DIR}/arrow-src/cpp/
@@ -635,6 +640,7 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD)
 		endif()
 
 		target_link_libraries(psp tbb)
+
 		target_link_libraries(binding tbb)
 
 		target_link_libraries(binding psp)

cpp/perspective/src/cpp/computed.cpp

@@ -1043,7 +1043,7 @@ t_computed_column::computed_functions = {
         {"category", "FunctionTokenType"},
         {"num_params", "1"},
         {"format_function", "x => `hour_of_day(${x})`"},
-        {"help", "Returns the hour of day (0-23) in UTC for the datetime column."},
+        {"help", "Returns the hour of day (0-23) for the datetime column."},
         {"signature", "hour_of_day(x: Datetime): Number"}
     }},
     {"day_of_week", {
@@ -1056,7 +1056,7 @@ t_computed_column::computed_functions = {
         {"category", "FunctionTokenType"},
         {"num_params", "1"},
         {"format_function", "x => `day_of_week(${x})`"},
-        {"help", "Returns the day of week in UTC for the datetime column."},
+        {"help", "Returns the day of week for the datetime column."},
         {"signature", "day_of_week(x: Datetime): String"}
     }},
     {"month_of_year", {
@@ -1069,7 +1069,7 @@ t_computed_column::computed_functions = {
         {"category", "FunctionTokenType"},
         {"num_params", "1"},
         {"format_function", "x => `month_of_year(${x})`"},
-        {"help", "Returns the month of year in UTC for the datetime column."},
+        {"help", "Returns the month of year for the datetime column."},
         {"signature", "month_of_year(x: Datetime): String"}
     }},
     {"second_bucket", {

cpp/perspective/src/cpp/computed_function.cpp

@@ -527,14 +527,13 @@ t_tscalar hour_of_day<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
-    date::sys_days days = date::floor<date::days>(ts);
-
-    // Subtract the day-precision `time_point` from the datetime-precision one
-    auto time_of_day = date::make_time(ts - days);
+    // Use localtime so that the hour of day is consistent with all output
+    // datetimes, which are in local time
+    std::time_t temp = std::chrono::system_clock::to_time_t(ts);
+    std::tm* t = std::localtime(&temp);
 
-    // Get the hour from the resulting `time_point`
-    rval.set(static_cast<std::int64_t>(time_of_day.hours().count()));
+    // Get the hour from the resulting `std::tm`
+    rval.set(static_cast<std::int64_t>(t->tm_hour));
     return rval;
 }
 
@@ -831,14 +830,14 @@ void day_of_week<DTYPE_TIME>(
     // 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);
-
-    // Find the weekday and write it to the output column
-    auto weekday = date::year_month_weekday(days).weekday_indexed().weekday();
+    // Use localtime so that the hour of day is consistent with all output
+    // datetimes, which are in local time
+    std::time_t temp = std::chrono::system_clock::to_time_t(ts);
+    std::tm* t = std::localtime(&temp);
 
+    // Get the weekday from the resulting `std::tm`
     output_column->set_nth(
-        idx, days_of_week[(weekday - date::Sunday).count()]);
+        idx, days_of_week[t->tm_wday]);
 }
 
 template <>
@@ -871,18 +870,16 @@ void month_of_year<DTYPE_TIME>(
     // 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);
-
-    // Cast the `time_point` to contain year/month/day
-    auto ymd = date::year_month_day(days);
+    // Use localtime so that the hour of day is consistent with all output
+    // datetimes, which are in local time
+    std::time_t temp = std::chrono::system_clock::to_time_t(ts);
+    std::tm* t = std::localtime(&temp);
 
-    // Get the month as an integer from 0 to 11
-    auto month = (ymd.month() -  date::January).count();
+    // Get the month from the resulting `std::tm`
+    auto month = t->tm_mon;
 
     // Get the month string and write into the output column
-    std::string month_of_year = months_of_year[month];
-    output_column->set_nth(idx, month_of_year);
+    output_column->set_nth(idx, months_of_year[month]);
 }
 
 } // end namespace computed_function

cpp/perspective/src/cpp/scalar.cpp

@@ -679,9 +679,23 @@ t_tscalar::to_string(bool for_expr) const {
             return ss.str();
         } break;
         case DTYPE_TIME: {
+            // Convert a millisecond UTC timestamp to a formatted datestring in
+            // local time, as all datetimes exported to the user happens in
+            // local time and not UTC.
             std::chrono::milliseconds timestamp(to_int64());
             date::sys_time<std::chrono::milliseconds> ts(timestamp);
-            ss << date::format("%Y-%m-%d %H:%M:%S", ts);
+            std::time_t temp = std::chrono::system_clock::to_time_t(ts);
+            std::tm* t = std::localtime(&temp);
+
+            // use a mix of std::put_time and date::format to properly
+            // represent datetimes to millisecond precision
+            ss << std::put_time(t, "%Y-%m-%d %H:%M:"); // ymd h:m
+
+            // TODO: we currently can't print out millisecond precision, but
+            // we need to.
+            ss << date::format("%S", ts); // represent second and millisecond
+            ss << std::put_time(t, " %Z"); // timezone
+
             return ss.str();
         } break;
         case DTYPE_STR: {

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

@@ -30,6 +30,7 @@
 #include <fstream>
 #include <boost/unordered_map.hpp>
 #include <perspective/portable.h>
+#include <stdlib.h>
 
 namespace perspective {
 

docs/i18n/en.json

@@ -35,15 +35,6 @@
       "node_modules/react/README": {
         "title": "node_modules/react/README"
       },
-      "obj/perspective-python": {
-        "title": "perspective-python API"
-      },
-      "obj/perspective-viewer": {
-        "title": "perspective-viewer API"
-      },
-      "obj/perspective": {
-        "title": "perspective API"
-      },
       "README": {
         "title": "README"
       }

docs/md/python.md

@@ -159,18 +159,18 @@ two methods into your object:
 
 #### Time Zone Handling
 
-When passing in `datetime` objects, Perspective checks the `tzinfo` attribute
-to see if a time zone is set. For more details, see this in-depth [explanation](https://github.com/finos/perspective/pull/867)
-of `perspective-python` semantics around time zone handling.
+Columns with the `datetime` type are stored internally as UTC timestamps in milliseconds since epoch (Unix Time),
+and are serialized to the user as `datetime.datetime` objects in _local time_ according to the Python runtime.
 
-##### Naive Datetimes
+Both ["naive" and "aware" datetimes](https://docs.python.org/3/library/datetime.html#aware-and-naive-objects) will be
+serialized to local time by Perspective, with the conversion determined by the `tzinfo` attribute:
 
-Objects with an unset `tzinfo` attribute (naive datetimes) are treated as _local time_, and do not undergo any time zone conversion.
+- "Naive" datetimes are assumed to be already in local time and are serialized as-is.
+- "Aware" datetimes will be converted to UTC from their original timezone, and then converted to local time
+from UTC.
 
-##### Aware Datetimes
-
-Objects with the `tzinfo` attribute set (aware datetimes) will be _converted into UTC_ before being stored in
-Perspective, and they will be _serialized as local time_.
+This behavior is consistent with Perspective's behavior in Javascript. For more details, see this
+in-depth [explanation](https://github.com/finos/perspective/pull/867) of `perspective-python` semantics around time zone handling.
 
 ##### Pandas Timestamps
 

packages/perspective-viewer/README.md

@@ -166,19 +166,19 @@ Sets new computed columns for the viewer.
 **Params**
 
 - computed-columns <code>[ &#x27;Array&#x27; ].&lt;Object&gt;</code> - An Array of computed column objects,
-which have three properties: `name`, a column name for the new column,
+which have three properties: `column`, a column name for the new column,
 `computed_function_name`, a String representing the computed function to
 apply, and `inputs`, an Array of String column names to be used as
 inputs to the computation.
 
 **Example** *(via Javascript DOM)*  
 ```js
 let elem = document.getElementById('my_viewer');
-elem.setAttribute('computed-columns', JSON.stringify([{name: "x+y", computed_function_name: "+", inputs: ["x", "y"]}]));
+elem.setAttribute('computed-columns', JSON.stringify([{column: "x+y", computed_function_name: "+", inputs: ["x", "y"]}]));
 ```
 **Example** *(via HTML)*  
 ```js
-<perspective-viewer computed-columns="[{name:'x+y',computed_function_name:'+',inputs:['x','y']}]""></perspective-viewer>
+<perspective-viewer computed-columns="[{column:'x+y',computed_function_name:'+',inputs:['x','y']}]""></perspective-viewer>
 ```
 
 * * *

packages/perspective-viewer/src/js/autocomplete_widget.js

@@ -218,8 +218,7 @@ class PerspectiveAutocompleteWidget extends HTMLElement {
 
         if (idx > -1 && children.length > 0) {
             children[idx].setAttribute("aria-selected", "true");
-            children[idx].scrollIntoView(true, {
-                behavior: "smooth",
+            children[idx].scrollIntoView({
                 block: "nearest"
             });
 

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

@@ -182,16 +182,16 @@ class PerspectiveViewer extends ActionElement {
      * @kind member
      * @type {Array<Object>}
      * @param {Array<Object>} computed-columns An Array of computed column objects,
-     * which have three properties: `name`, a column name for the new column,
+     * which have three properties: `column`, a column name for the new column,
      * `computed_function_name`, a String representing the computed function to
      * apply, and `inputs`, an Array of String column names to be used as
      * inputs to the computation.
      * @fires PerspectiveViewer#perspective-config-update
      * @example <caption>via Javascript DOM</caption>
      * let elem = document.getElementById('my_viewer');
-     * elem.setAttribute('computed-columns', JSON.stringify([{name: "x+y", computed_function_name: "+", inputs: ["x", "y"]}]));
+     * elem.setAttribute('computed-columns', JSON.stringify([{column: "x+y", computed_function_name: "+", inputs: ["x", "y"]}]));
      * @example <caption>via HTML</caption>
-     * <perspective-viewer computed-columns="[{name:'x+y',computed_function_name:'+',inputs:['x','y']}]""></perspective-viewer>
+     * <perspective-viewer computed-columns="[{column:'x+y',computed_function_name:'+',inputs:['x','y']}]""></perspective-viewer>
      */
     @array_attribute
     "computed-columns"(computed_columns) {

packages/perspective-viewer/src/js/viewer/perspective_element.js

@@ -182,10 +182,10 @@ export class PerspectiveElement extends StateElement {
 
         if (parsed_computed_columns.length === 0) {
             // Fallback for race condition on workspace - need to parse
-            // computed expressions, and assume that `parsed-computed-columns`
-            // will be set when the setAttribute callback fires
-            // *after* the table has been loaded.
+            // computed expressions and then set `parsed-computed-columns`
+            // so that future views can get the computed column.
             const computed_expressions = this._get_view_computed_columns();
+
             for (const expression of computed_expressions) {
                 if (typeof expression === "string") {
                     parsed_computed_columns = parsed_computed_columns.concat(this._computed_expression_parser.parse(expression));
@@ -195,9 +195,17 @@ export class PerspectiveElement extends StateElement {
             }
         }
 
-        const computed_column_names = parsed_computed_columns.map(x => x.column);
         const computed_schema = await table.computed_schema(parsed_computed_columns);
 
+        // Validate the computed columns and make sure no invalid columns
+        // are present, as invalid columns can cause segfaults later on.
+        const validated = await this._validate_parsed_computed_columns(parsed_computed_columns, computed_schema);
+        parsed_computed_columns = validated;
+
+        // Update the viewer with the parsed computed columns
+        this.setAttribute("parsed-computed-columns", JSON.stringify(parsed_computed_columns));
+
+        const computed_column_names = parsed_computed_columns.map(x => x.column);
         cols = cols.concat(computed_column_names);
 
         if (!this.hasAttribute("columns")) {

packages/perspective-viewer/test/js/unit/computed_expressions_parser.spec.js

@@ -41,6 +41,23 @@ describe("Computed Expression Parser", function() {
         expect(parsed).toEqual(expected);
     });
 
+    it.skip("Should parse an operator notation expression with associativity", function() {
+        const expected = [
+            {
+                column: "(w + x)",
+                computed_function_name: "+",
+                inputs: ["w", "x"]
+            },
+            {
+                column: "((w + x) + z)",
+                computed_function_name: "+",
+                inputs: ["(w + x)", "z"]
+            }
+        ];
+        const parsed = COMPUTED_EXPRESSION_PARSER.parse('"w" + "x" + "z"');
+        expect(parsed).toEqual(expected);
+    });
+
     it("Should parse an operator notation expression named with 'AS'", function() {
         const expected = [
             {

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

@@ -8,15 +8,15 @@
     "Computed_Expressions_Typing_a_datetime_function_should_show_autocomplete_for_datetime_columns": "d88e3c1958db28913986caa9b52bc9af",
     "Computed_Expressions_Typing_a_partial_column_name_should_show_autocomplete": "dc6bc237afed37845241281d9e7013d7",
     "Computed_Expressions_Typing_a_long_expression_should_dock_the_autocomplete": "d8bbc17948371907e494b76c1e23f2b5",
-    "Computed_Expressions_Typing_a_long_expression_should_dock_the_autocomplete,_and_the_details_panel_should_show": "35335cde3e70ad743da4b544500ccf71",
+    "Computed_Expressions_Typing_a_long_expression_should_dock_the_autocomplete,_and_the_details_panel_should_show": "62688ada39c92285ac2c1b596155d3b8",
     "Computed_Expressions_Typing_an_expression_in_the_textarea_should_work_even_when_pushed_down_to_page_bottom_": "c5f3de03cb95f581a31cdbb37acadbdf",
     "Computed_Expressions_Typing_enter_should_save_a_valid_expression": "409fb88c2373dceb480991f6f5f985fc",
     "Computed_Expressions_Typing_enter_should_not_save_an_invalid_expression": "266ffcd4766f1505bf925d7ba5ce583f",
     "Computed_Expressions_Pressing_arrow_down_should_select_the_next_autocomplete_item": "e2ba1f9f03f7e046dd2309fc04b6433e",
     "Computed_Expressions_Pressing_arrow_down_on_the_last_item_should_select_the_first_autocomplete_item": "67c477a9515f85734ba044c8f4d68eec",
     "Computed_Expressions_Pressing_arrow_up_should_select_the_previous_autocomplete_item": "67c477a9515f85734ba044c8f4d68eec",
     "Computed_Expressions_Pressing_arrow_up_from_the_first_item_should_select_the_last_autocomplete_item": "d5ac2a245dd20f99d8b1bbab292c66c6",
-    "Computed_Expressions_Pressing_arrow_down_on_an_undocked_autocomplete_should_select_the_next_autocomplete_item": "f9e63e4c0aa5307543360abbfe2e09c1",
+    "Computed_Expressions_Pressing_arrow_down_on_an_undocked_autocomplete_should_select_the_next_autocomplete_item": "32cc425a0c1426805776307befa4808c",
     "Computed_Expressions_Pressing_arrow_down_on_the_last_item_on_an_undocked_autocomplete_should_select_the_first_autocomplete_item": "a87b2a4caeb89462218838e0aa4b1c99",
     "Computed_Expressions_Pressing_arrow_up_on_an_undocked_autocomplete_should_select_the_previous_autocomplete_item": "9fb08a9f61a7624be9c5440c15d0e959",
     "Computed_Expressions_Pressing_arrow_up_from_the_first_item_on_an_undocked_autocomplete_should_select_the_last_autocomplete_item": "9fb08a9f61a7624be9c5440c15d0e959",

packages/perspective/README.md

@@ -171,7 +171,7 @@ columns in the underlying [table](#module_perspective..table).
 // Create a view with computed columns
 const view = table.view({
      computed_columns: [{
-         name: "x + y",
+         column: "x + y",
          computed_function_name: "+",
          inputs: ["x", "y"]
      }]

packages/perspective/src/js/perspective.js

@@ -293,7 +293,7 @@ export default function(Module) {
      * // Create a view with computed columns
      * const view = table.view({
      *      computed_columns: [{
-     *          name: "x + y",
+     *          column: "x + y",
      *          computed_function_name: "+",
      *          inputs: ["x", "y"]
      *      }]
@@ -887,6 +887,11 @@ export default function(Module) {
      * Unregister a previously registered update callback with this
      * {@link module:perspective~view}.
      *
+     * @example
+     * // remove an `on_update` callback
+     * const callback = updated => console.log(updated);
+     * view.remove_update(callback);
+     *
      * @param {function} callback A update callback function to be removed
      */
     view.prototype.remove_update = function(callback) {
@@ -901,6 +906,10 @@ export default function(Module) {
      * the {@link module:perspective~view} is deleted, this callback will be
      * invoked.
      *
+     * @example
+     * // attach an `on_delete` callback
+     * view.on_delete(() => console.log("Deleted!"));
+     *
      * @param {function} callback A callback function invoked on delete.
      */
     view.prototype.on_delete = function(callback) {
@@ -911,6 +920,11 @@ export default function(Module) {
      * Unregister a previously registered delete callback with this
      * {@link module:perspective~view}.
      *
+     * @example
+     * // remove an `on_delete` callback
+     * const callback = () => console.log("Deleted!")
+     * view.remove_delete(callback);
+     *
      * @param {function} callback A delete callback function to be removed
      */
     view.prototype.remove_delete = function(callback) {

python/perspective/perspective/include/perspective/python/column.h

@@ -12,7 +12,6 @@
 #include <perspective/first.h>
 #include <perspective/column.h>
 #include <perspective/base.h>
-#include <perspective/python/column.h>
 #include <perspective/python/base.h>
 
 namespace perspective {