From 6a8979572f7c1ca4959c00c2a2cbac5119160cc4 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Tue, 28 Aug 2018 13:30:51 -0400 Subject: [PATCH 1/4] Moved first/last tests to proper section --- packages/perspective/test/js/pivots.js | 115 +++++++++++++------------ 1 file changed, 58 insertions(+), 57 deletions(-) diff --git a/packages/perspective/test/js/pivots.js b/packages/perspective/test/js/pivots.js index 3995191564..f44c77ef37 100644 --- a/packages/perspective/test/js/pivots.js +++ b/packages/perspective/test/js/pivots.js @@ -94,6 +94,64 @@ module.exports = (perspective) => { expect(answer).toEqual(result); }); + it("['z'], first by index", async function () { + var table = perspective.table(data); + var view = table.view({ + row_pivot: ['z'], + aggregate: [{op: 'first by index', column:'x'}], + }); + var answer = [ + {__ROW_PATH__: [], x: 1}, + {__ROW_PATH__: [ false ], x: 2}, + {__ROW_PATH__: [ true ], x: 1}, + ]; + let result = await view.to_json(); + expect(answer).toEqual(result); + }); + + it("['z'], last by index", async function () { + var table = perspective.table(data); + var view = table.view({ + row_pivot: ['z'], + aggregate: [{op: 'last by index', column:'x'}], + }); + var answer = [ + {__ROW_PATH__: [], x: 4}, + {__ROW_PATH__: [ false ], x: 4}, + {__ROW_PATH__: [ true ], x: 3}, + ]; + let result = await view.to_json(); + expect(answer).toEqual(result); + }); + + it("['z'], last", async function () { + var table = perspective.table(data); + var view = table.view({ + row_pivot: ['z'], + aggregate: [{op: 'last', column:'x'}], + }); + var answer = [ + {__ROW_PATH__: [], x: 3}, + {__ROW_PATH__: [ false ], x: 4}, + {__ROW_PATH__: [ true ], x: 3}, + ]; + let result = await view.to_json(); + expect(answer).toEqual(result); + + table.update([ + {'x': 1, 'y':'c', 'z': true}, + {'x': 2, 'y':'d', 'z': false} + ]); + var answerAfterUpdate = [ + {__ROW_PATH__: [], x: 1}, + {__ROW_PATH__: [ false ], x: 2}, + {__ROW_PATH__: [ true ], x: 1}, + ]; + let result2 = await view.to_json(); + expect(answerAfterUpdate).toEqual(result2); + }); + + }); describe("Aggregates with nulls", function () { @@ -203,63 +261,6 @@ module.exports = (perspective) => { expect(answer).toEqual(result); }); - it("['z'], first by index", async function () { - var table = perspective.table(data); - var view = table.view({ - row_pivot: ['z'], - aggregate: [{op: 'first by index', column:'x'}], - }); - var answer = [ - {__ROW_PATH__: [], x: 1}, - {__ROW_PATH__: [ false ], x: 2}, - {__ROW_PATH__: [ true ], x: 1}, - ]; - let result = await view.to_json(); - expect(answer).toEqual(result); - }); - - it("['z'], last by index", async function () { - var table = perspective.table(data); - var view = table.view({ - row_pivot: ['z'], - aggregate: [{op: 'last by index', column:'x'}], - }); - var answer = [ - {__ROW_PATH__: [], x: 4}, - {__ROW_PATH__: [ false ], x: 4}, - {__ROW_PATH__: [ true ], x: 3}, - ]; - let result = await view.to_json(); - expect(answer).toEqual(result); - }); - - it("['z'], last", async function () { - var table = perspective.table(data); - var view = table.view({ - row_pivot: ['z'], - aggregate: [{op: 'last', column:'x'}], - }); - var answer = [ - {__ROW_PATH__: [], x: 3}, - {__ROW_PATH__: [ false ], x: 4}, - {__ROW_PATH__: [ true ], x: 3}, - ]; - let result = await view.to_json(); - expect(answer).toEqual(result); - - table.update([ - {'x': 1, 'y':'c', 'z': true}, - {'x': 2, 'y':'d', 'z': false} - ]); - var answerAfterUpdate = [ - {__ROW_PATH__: [], x: 1}, - {__ROW_PATH__: [ false ], x: 2}, - {__ROW_PATH__: [ true ], x: 1}, - ]; - let result2 = await view.to_json(); - expect(answerAfterUpdate).toEqual(result2); - }); - }); describe("Row pivot", function() { From 5f2a8d6c9ae65ac7f9b935b8cb30dba249cb2424 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Tue, 28 Aug 2018 13:47:52 -0400 Subject: [PATCH 2/4] Fixed bug in mean aggregate which skipped 0 in addition to unset. --- packages/perspective/src/cpp/gnode_state.cpp | 44 +++++++++++++++---- packages/perspective/src/cpp/main.cpp | 2 +- packages/perspective/src/cpp/sparse_tree.cpp | 17 ++----- .../src/include/perspective/gnode_state.h | 5 +++ packages/perspective/src/js/perspective.js | 5 ++- packages/perspective/test/js/pivots.js | 28 +++++++++++- 6 files changed, 74 insertions(+), 27 deletions(-) diff --git a/packages/perspective/src/cpp/gnode_state.cpp b/packages/perspective/src/cpp/gnode_state.cpp index 79edbb7d05..e9feeee67d 100644 --- a/packages/perspective/src/cpp/gnode_state.cpp +++ b/packages/perspective/src/cpp/gnode_state.cpp @@ -490,22 +490,50 @@ void t_gstate::read_column(const t_str& colname, const t_tscalvec& pkeys, t_f64vec& out_data) const +{ + read_column(colname, pkeys, out_data, true); +} + +void +t_gstate::read_column(const t_str& colname, + const t_tscalvec& pkeys, + t_f64vec& out_data, + bool include_nones) const { t_index num = pkeys.size(); t_col_csptr col = m_table->get_const_column(colname); const t_column* col_ = col.get(); - t_f64vec rval(num); - - for (t_index idx = 0; idx < num; ++idx) + + if (include_nones) { - t_mapping::const_iterator iter = m_mapping.find(pkeys[idx]); - if (iter != m_mapping.end()) + t_f64vec rval(num); + for (t_index idx = 0; idx < num; ++idx) { - rval[idx] = col_->get_scalar(iter->second).to_double(); + t_mapping::const_iterator iter = m_mapping.find(pkeys[idx]); + if (iter != m_mapping.end()) + { + rval[idx] = col_->get_scalar(iter->second).to_double(); + } } + std::swap(rval, out_data); } - - std::swap(rval, out_data); + else + { + t_f64vec rval; + for (t_index idx = 0; idx < num; ++idx) + { + t_mapping::const_iterator iter = m_mapping.find(pkeys[idx]); + if (iter != m_mapping.end()) + { + auto tscalar = col_->get_scalar(iter->second); + if (tscalar.is_valid()) + { + rval.push_back(tscalar.to_double()); + } + } + } + std::swap(rval, out_data); + } } t_tscalar diff --git a/packages/perspective/src/cpp/main.cpp b/packages/perspective/src/cpp/main.cpp index 4eb9249435..8cf135f8df 100644 --- a/packages/perspective/src/cpp/main.cpp +++ b/packages/perspective/src/cpp/main.cpp @@ -256,7 +256,7 @@ _fill_col(val dcol, t_col_sptr col, t_bool is_arrow) } else { for (auto i = 0; i < nrows; ++i) { - if (dcol[i].isUndefined()) continue; + if (dcol[i].isUndefined() || dcol[i].isNull()) continue; auto elem = dcol[i].as(); col->set_nth(i, elem); } diff --git a/packages/perspective/src/cpp/sparse_tree.cpp b/packages/perspective/src/cpp/sparse_tree.cpp index d53575b8c3..d113246073 100644 --- a/packages/perspective/src/cpp/sparse_tree.cpp +++ b/packages/perspective/src/cpp/sparse_tree.cpp @@ -1204,22 +1204,11 @@ t_stree::update_agg_table(t_uindex nidx, t_f64vec values; gstate.read_column( - spec.get_dependencies()[0].name(), pkeys, values); + spec.get_dependencies()[0].name(), pkeys, values, false); - t_float64 nr = 0; - t_float64 dr = 0; - auto bit = values.begin(); - auto eit = values.end(); + auto nr = std::accumulate(values.begin(), values.end(), t_float64(0)); + t_float64 dr = values.size(); - for(; bit != eit; ++bit) - { - auto val = *bit; - if (val != NULL) - { - nr += val; - dr += 1; - } - } t_f64pair* dst_pair = dst->get_nth(dst_ridx); diff --git a/packages/perspective/src/include/perspective/gnode_state.h b/packages/perspective/src/include/perspective/gnode_state.h index 10cab0baab..423b0790ca 100644 --- a/packages/perspective/src/include/perspective/gnode_state.h +++ b/packages/perspective/src/include/perspective/gnode_state.h @@ -56,6 +56,11 @@ class PERSPECTIVE_EXPORT t_gstate const t_tscalvec& pkeys, t_f64vec& out_data) const; + void read_column(const t_str& colname, + const t_tscalvec& pkeys, + t_f64vec& out_data, + bool include_nones) const; + t_table_sptr get_table(); t_table_csptr get_table() const; diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js index 671e311485..c46522d2f3 100644 --- a/packages/perspective/src/js/perspective.js +++ b/packages/perspective/src/js/perspective.js @@ -186,8 +186,9 @@ function parse_data(data, names, types) { if (inferredType.value === __MODULE__.t_dtype.DTYPE_FLOAT64.value) { col.push(Number(data[x][name])); } else if (inferredType.value === __MODULE__.t_dtype.DTYPE_INT32.value) { - const val = Number(data[x][name]); - col.push(val); + let val = data[x][name]; + if (val !== null) val = Number(val); + col.push(val); if (val > 2147483647 || val < -2147483648) { types[n] = __MODULE__.t_dtype.DTYPE_FLOAT64; } diff --git a/packages/perspective/test/js/pivots.js b/packages/perspective/test/js/pivots.js index f44c77ef37..c3f0562db4 100644 --- a/packages/perspective/test/js/pivots.js +++ b/packages/perspective/test/js/pivots.js @@ -156,7 +156,7 @@ module.exports = (perspective) => { describe("Aggregates with nulls", function () { - it("Mean", async function () { + it("mean", async function () { var table = perspective.table([ {'x': 3, 'y':1}, {'x': 2, 'y':1}, @@ -169,7 +169,7 @@ module.exports = (perspective) => { row_pivot: ['y'], aggregate: [{op: 'mean', column:'x'}], }); - var answer = [ + var answer = [ {__ROW_PATH__: [], x: 3}, {__ROW_PATH__: [ 1 ], x: 2.5}, {__ROW_PATH__: [ 2 ], x: 4}, @@ -178,6 +178,30 @@ module.exports = (perspective) => { expect(answer).toEqual(result); }); + it("mean with 0", async function () { + var table = perspective.table([ + {'x': 3, 'y':1}, + {'x': 3, 'y':1}, + {'x': 0, 'y':1}, + {'x': null, 'y':1}, + {'x': null, 'y':1}, + {'x': 4, 'y':2}, + {'x': null, 'y':2} + ]); + var view = table.view({ + row_pivot: ['y'], + aggregate: [{op: 'mean', column:'x'}], + }); + var answer = [ + {__ROW_PATH__: [], x: 2.5}, + {__ROW_PATH__: [ 1 ], x: 2}, + {__ROW_PATH__: [ 2 ], x: 4}, + ]; + let result = await view.to_json(); + expect(answer).toEqual(result); + }); + + it("sum", async function () { var table = perspective.table([ {'x': 3, 'y':1}, From ba55cc7ffcbfbf88d734762727ae66ee7df039c8 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Tue, 28 Aug 2018 15:45:47 -0400 Subject: [PATCH 3/4] Fixed null aggregates for floats as well --- packages/perspective/src/js/perspective.js | 6 +++++- packages/perspective/test/js/pivots.js | 23 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js index c46522d2f3..c646e3d83e 100644 --- a/packages/perspective/src/js/perspective.js +++ b/packages/perspective/src/js/perspective.js @@ -184,7 +184,11 @@ function parse_data(data, names, types) { continue; }; if (inferredType.value === __MODULE__.t_dtype.DTYPE_FLOAT64.value) { - col.push(Number(data[x][name])); + let val = data[x][name]; + if (val !== null) { + val = Number(val); + } + col.push(val); } else if (inferredType.value === __MODULE__.t_dtype.DTYPE_INT32.value) { let val = data[x][name]; if (val !== null) val = Number(val); diff --git a/packages/perspective/test/js/pivots.js b/packages/perspective/test/js/pivots.js index c3f0562db4..9bf162ab3e 100644 --- a/packages/perspective/test/js/pivots.js +++ b/packages/perspective/test/js/pivots.js @@ -201,6 +201,29 @@ module.exports = (perspective) => { expect(answer).toEqual(result); }); + it("mean with 0.0 (floats)", async function () { + var table = perspective.table({x: 'float', y: 'integer'}); + table.update([ + {'x': 3, 'y':1}, + {'x': 3, 'y':1}, + {'x': 0, 'y':1}, + {'x': null, 'y':1}, + {'x': null, 'y':1}, + {'x': 4, 'y':2}, + {'x': null, 'y':2} + ]); + var view = table.view({ + row_pivot: ['y'], + aggregate: [{op: 'mean', column:'x'}], + }); + var answer = [ + {__ROW_PATH__: [], x: 2.5}, + {__ROW_PATH__: [ 1 ], x: 2}, + {__ROW_PATH__: [ 2 ], x: 4}, + ]; + let result = await view.to_json(); + expect(answer).toEqual(result); + }); it("sum", async function () { var table = perspective.table([ From 7454dc0a76508bbe72a7adf1d8d8fd1f8eded3d2 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Tue, 28 Aug 2018 16:08:42 -0400 Subject: [PATCH 4/4] Added tests for filters in presence of null --- packages/perspective/test/js/filters.js | 73 ++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/perspective/test/js/filters.js b/packages/perspective/test/js/filters.js index 26c9094511..6af0da7c09 100644 --- a/packages/perspective/test/js/filters.js +++ b/packages/perspective/test/js/filters.js @@ -213,8 +213,79 @@ module.exports = (perspective) => { }); - }); + describe("nulls", function () { + + it("x > 2", async function () { + var table = perspective.table([ + {'x': 3, 'y':1}, + {'x': 2, 'y':1}, + {'x': null, 'y':1}, + {'x': null, 'y':1}, + {'x': 4, 'y':2}, + {'x': null, 'y':2} + ]); + var view = table.view({ + filter: [ + ['x', '>', 2] + ] + }); + var answer = [ + {'x': 3, 'y':1}, + {'x': 4, 'y':2}, + ]; + let result = await view.to_json(); + expect(result).toEqual(answer); + }); + + + it("x < 3", async function () { + var table = perspective.table([ + {'x': 3, 'y':1}, + {'x': 2, 'y':1}, + {'x': null, 'y':1}, + {'x': null, 'y':1}, + {'x': 4, 'y':2}, + {'x': null, 'y':2} + ]); + var view = table.view({ + filter: [ + ['x', '<', 3] + ] + }); + var answer = [ + {'x': 2, 'y':1}, + ]; + let result = await view.to_json(); + expect(result).toEqual(answer); + }); + + + it("x > 2", async function () { + var table = perspective.table({x: 'float', y: 'integer'}); + table.update([ + {'x': 3.5, 'y':1}, + {'x': 2.5, 'y':1}, + {'x': null, 'y':1}, + {'x': null, 'y':1}, + {'x': 4.5, 'y':2}, + {'x': null, 'y':2} + ]); + var view = table.view({ + filter: [ + ['x', '>', 2.5] + ] + }); + var answer = [ + {'x': 3.5, 'y':1}, + {'x': 4.5, 'y':2}, + ]; + let result = await view.to_json(); + expect(answer).toEqual(result); + }); + + }); + }); };