Fixed bug in mean aggregate which skipped 0 in addition to unset.

finos · Aug 28, 2018 · 5f2a8d6 · 5f2a8d6
1 parent 6a89795
commit 5f2a8d6
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 27 deletions.
diff --git 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
@@ -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<T>();
             col->set_nth(i, elem);
         }

diff --git 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<t_f64pair>(dst_ridx);

diff --git 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
@@ -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
@@ -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},