Merge pull request #2545 from finos/fix-split-by-date

Fix `split_by` with `date`/`datetime` columns
finos · Feb 26, 2024 · fda23d8 · fda23d8
2 parents c4b5c01 + 7c70021
commit fda23d8
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 5 deletions.
diff --git a/cpp/perspective/src/cpp/emscripten.cpp b/cpp/perspective/src/cpp/emscripten.cpp
@@ -2141,7 +2141,7 @@ EMSCRIPTEN_BINDINGS(perspective) {
         .function("schema", &View<t_ctxunit>::schema)
         .function("expression_schema", &View<t_ctxunit>::expression_schema)
         .function("column_names", &View<t_ctxunit>::column_names)
-        .function("column_paths", &View<t_ctxunit>::column_paths)
+        .function("column_paths", &View<t_ctxunit>::column_paths_string)
         .function("_get_deltas_enabled", &View<t_ctxunit>::_get_deltas_enabled)
         .function("_set_deltas_enabled", &View<t_ctxunit>::_set_deltas_enabled)
         .function(
@@ -2172,7 +2172,7 @@ EMSCRIPTEN_BINDINGS(perspective) {
         .function("schema", &View<t_ctx0>::schema)
         .function("expression_schema", &View<t_ctx0>::expression_schema)
         .function("column_names", &View<t_ctx0>::column_names)
-        .function("column_paths", &View<t_ctx0>::column_paths)
+        .function("column_paths", &View<t_ctx0>::column_paths_string)
         .function("_get_deltas_enabled", &View<t_ctx0>::_get_deltas_enabled)
         .function("_set_deltas_enabled", &View<t_ctx0>::_set_deltas_enabled)
         .function(
@@ -2207,7 +2207,7 @@ EMSCRIPTEN_BINDINGS(perspective) {
         .function("schema", &View<t_ctx1>::schema)
         .function("expression_schema", &View<t_ctx1>::expression_schema)
         .function("column_names", &View<t_ctx1>::column_names)
-        .function("column_paths", &View<t_ctx1>::column_paths)
+        .function("column_paths", &View<t_ctx1>::column_paths_string)
         .function("_get_deltas_enabled", &View<t_ctx1>::_get_deltas_enabled)
         .function("_set_deltas_enabled", &View<t_ctx1>::_set_deltas_enabled)
         .function(
@@ -2241,7 +2241,7 @@ EMSCRIPTEN_BINDINGS(perspective) {
         .function("schema", &View<t_ctx2>::schema)
         .function("expression_schema", &View<t_ctx2>::expression_schema)
         .function("column_names", &View<t_ctx2>::column_names)
-        .function("column_paths", &View<t_ctx2>::column_paths)
+        .function("column_paths", &View<t_ctx2>::column_paths_string)
         .function("_get_deltas_enabled", &View<t_ctx2>::_get_deltas_enabled)
         .function("_set_deltas_enabled", &View<t_ctx2>::_set_deltas_enabled)
         .function(

diff --git a/cpp/perspective/src/cpp/view.cpp b/cpp/perspective/src/cpp/view.cpp
@@ -270,6 +270,25 @@ View<CTX_T>::column_paths() const {
     return names;
 }
 
+template <typename CTX_T>
+std::vector<std::vector<std::string>>
+View<CTX_T>::column_paths_string() const {
+    auto paths = column_paths();
+    std::vector<std::vector<std::string>> out;
+    out.reserve(paths.size());
+    for (const auto& path : paths) {
+        std::vector<std::string> row;
+        row.reserve(path.size());
+        for (const auto& c : path) {
+            row.push_back(c.to_string());
+        }
+
+        out.push_back(row);
+    }
+
+    return out;
+}
+
 template <typename CTX_T>
 std::map<std::string, std::string>
 View<CTX_T>::schema() const {

diff --git a/cpp/perspective/src/include/perspective/view.h b/cpp/perspective/src/include/perspective/view.h
@@ -125,6 +125,8 @@ class PERSPECTIVE_EXPORT View {
      */
     std::vector<std::vector<t_tscalar>> column_paths() const;
 
+    std::vector<std::vector<std::string>> column_paths_string() const;
+
     /**
      * @brief
      *

diff --git a/packages/perspective-viewer-datagrid/test/js/superstore.spec.js b/packages/perspective-viewer-datagrid/test/js/superstore.spec.js
@@ -62,4 +62,30 @@ test.describe("Datagrid with superstore data set", () => {
             "row-headers-are-printed-correctly"
         );
     });
+
+    test("Column headers are printed correctly, split_by a date column", async ({
+        page,
+    }) => {
+        await page.goto("/tools/perspective-test/src/html/basic-test.html");
+        await page.evaluate(async () => {
+            while (!window["__TEST_PERSPECTIVE_READY__"]) {
+                await new Promise((x) => setTimeout(x, 10));
+            }
+        });
+
+        await page.evaluate(async () => {
+            await document.querySelector("perspective-viewer").restore({
+                plugin: "Datagrid",
+                columns: ["Sales", "Profit"],
+                group_by: ["State"],
+                split_by: ["New Col"],
+                expressions: { "New Col": "bucket(\"Order Date\",'Y')" },
+            });
+        });
+
+        compareContentsToSnapshot(
+            await getDatagridContents(page),
+            "column-headers-are-printed-correctly-split-by-a-date-column"
+        );
+    });
 });
diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js
@@ -324,6 +324,25 @@ export default function (Module) {
         return extracted;
     };
 
+    const extract_vector_string = function (vector) {
+        // handles deletion already - do not call delete() on the input vector
+        // again
+        let extracted = [];
+        for (let i = 0; i < vector.size(); i++) {
+            let item = vector.get(i);
+            let row = [];
+            for (let i = 0; i < item.size(); i++) {
+                let s = item.get(i);
+                row.push(s);
+            }
+
+            item.delete();
+            extracted.push(row);
+        }
+        vector.delete();
+        return extracted;
+    };
+
     /**
      * The schema of this {@link module:perspective~view}.
      *
@@ -420,7 +439,7 @@ export default function (Module) {
      * @returns {Array<String>} an Array of Strings containing the column paths.
      */
     view.prototype.column_paths = function () {
-        return extract_vector_scalar(this._View.column_paths()).map((x) =>
+        return extract_vector_string(this._View.column_paths()).map((x) =>
             x.join(defaults.COLUMN_SEPARATOR_STRING)
         );
     };

diff --git a/packages/perspective/test/js/pivots.spec.js b/packages/perspective/test/js/pivots.spec.js
@@ -3082,5 +3082,32 @@ const std = (nums) => {
             view.delete();
             table.delete();
         });
+
+        test("Should format date columns in split_by", async function () {
+            const table = await perspective.table({
+                w: "float",
+                x: "integer",
+                y: "string",
+                z: "date",
+            });
+
+            await table.update(data_8);
+            const view = await table.view({ group_by: ["y"], split_by: ["z"] });
+            const paths = await view.column_paths();
+            expect(paths).toEqual([
+                "__ROW_PATH__",
+                "2019-04-11|w",
+                "2019-04-11|x",
+                "2019-04-11|y",
+                "2019-04-11|z",
+                "2019-04-13|w",
+                "2019-04-13|x",
+                "2019-04-13|y",
+                "2019-04-13|z",
+            ]);
+
+            view.delete();
+            table.delete();
+        });
     });
 })(perspective);
diff --git a/tools/perspective-test/results.tar.gz b/tools/perspective-test/results.tar.gz
-Original file line number
+Diff line change
@@ Expand Up / @@ -125,6 +125,8 @@ class PERSPECTIVE_EXPORT View { @@
          */
         std::vector<std::vector<t_tscalar>> column_paths() const;
+        std::vector<std::vector<std::string>> column_paths_string() const;
         /**
          * @brief
          *
@@ Expand Down @@