Merge pull request #523 from jpmorganchase/fix_column_only

Fix issue #522: row count is correct on column-only views
finos · Apr 5, 2019 · 16383ec · 16383ec
2 parents 1773e30 + 35e1b7b
commit 16383ec
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 3 deletions.
diff --git a/cpp/perspective/src/cpp/view.cpp b/cpp/perspective/src/cpp/view.cpp
@@ -62,7 +62,11 @@ View<t_ctx2>::sides() const {
 template <typename CTX_T>
 std::int32_t
 View<CTX_T>::num_rows() const {
-    return m_ctx->get_row_count();
+    if (is_column_only()) {
+        return m_ctx->get_row_count() - 1;
+    } else {
+        return m_ctx->get_row_count();
+    }
 }
 
 template <typename CTX_T>

diff --git a/packages/perspective-viewer-hypergrid/test/js/regressions.spec.js b/packages/perspective-viewer-hypergrid/test/js/regressions.spec.js
@@ -44,6 +44,11 @@ utils.with_server({}, () => {
         "regressions.html",
         () => {
             describe("Updates", () => {
+                test.capture("should not render an extra row for column_only views", async page => {
+                    const viewer = await page.$("perspective-viewer");
+                    await page.shadow_click("perspective-viewer", "#config_button");
+                    await page.evaluate(element => element.setAttribute("column-pivots", '["y"]'), viewer);
+                });
                 test.capture("regular updates", async page => {
                     const viewer = await page.$("perspective-viewer");
                     await page.shadow_click("perspective-viewer", "#config_button");

diff --git a/packages/perspective-viewer-hypergrid/test/results/results.json b/packages/perspective-viewer-hypergrid/test/results/results.json
@@ -1,5 +1,6 @@
 {
     "empty.html/empty grids do not explode": "423ca653bbcbc21a28029c149a37b8ec",
+    "regressions.html/should not render an extra row for column_only views": "0656c682240c18c13c1d5892bf790d8a",
     "regressions.html/regular updates": "14abd51c3cae1919119d9f88bfbc5cd3",
     "regressions.html/saving a computed column does not interrupt update rendering": "da13afb4284b9c3da21ed57c6ba69301",
     "superstore.html/shows a grid without any settings applied.": "59ecbb591317976232b7dc078b79e164",

diff --git a/packages/perspective/test/js/constructors.js b/packages/perspective/test/js/constructors.js
@@ -320,7 +320,7 @@ module.exports = perspective => {
             var view = table.view({column_pivot: ["float"]});
             const result = await view.col_to_js_typed_array("3.5|int");
             // bytelength should not include the aggregate row
-            expect(result[0].byteLength).toEqual(16);
+            expect(result[0].byteLength).toEqual(12);
             view.delete();
             table.delete();
         });
@@ -329,7 +329,7 @@ module.exports = perspective => {
             var table = perspective.table(int_float_data);
             var view = table.view({column_pivot: ["float"]});
             const result = await view.col_to_js_typed_array("3.5|float");
-            expect(result[0].byteLength).toEqual(32);
+            expect(result[0].byteLength).toEqual(24);
             view.delete();
             table.delete();
         });
@@ -523,6 +523,20 @@ module.exports = perspective => {
             table.delete();
         });
 
+        it("Returns the correct number of rows for column-only views", async function() {
+            var table = perspective.table(data);
+            var view = table.view();
+            var num_rows = await view.num_rows();
+            var view2 = table.view({
+                column_pivot: ["x"]
+            });
+            var num_rows_col_only = await view2.num_rows();
+            expect(num_rows_col_only).toEqual(num_rows);
+            view.delete();
+            view2.delete();
+            table.delete();
+        });
+
         it.skip("Handles inconsistent rows with same width", async function() {
             const int_to_float = [{x: 1, y: 2}, {y: 2, z: 3}];
             var table = perspective.table(int_to_float);