fix(aggregate): include first row in SUM and coerce numeric strings

README.md

@@ -61,6 +61,8 @@ Current supported `method`s (case insensitive):
 + 'min'
 + 'max'
 
+`sum` / `average` / `min` / `max` coerce cells to numbers (so `'400'` + `'100'` is `500`, not `'400100'`). A group with a single row sums to that row's value, not `0`.
+
 Also see this [example](https://echarts.apache.org/examples/en/editor.html?c=data-transform-aggregate).
 
 

package.json

@@ -13,6 +13,7 @@
     "build": "node build/build.js",
     "release": "node build/build.js --release",
     "lint": "./node_modules/.bin/eslint src/**/*.ts",
+    "test": "node test/aggregate-sum.test.js",
     "cp2echarts": "cp ./dist/ecSimpleTransform.js ../echarts/test/lib/ecSimpleTransform.js"
   },
   "dependencies": {

src/aggregate.ts

@@ -21,7 +21,7 @@ import {
     DataTransformOption, DimensionLoose, DimensionName, ExternalDataTransform,
     ExternalDimensionDefinition, ExternalSource, OptionDataValue
 } from './types';
-import { assert, hasOwn, quantile } from './util';
+import { assert, hasOwn, quantile, toNumber } from './util';
 
 /**
  * @usage
@@ -496,7 +496,10 @@ const lineCreator: {
     ) => OptionDataValue
 } = {
     'SUM'(upstream, dataIndex, dimInfo) {
-        return upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream);
+        // Seed with the first in-group value (not 0). Otherwise singleton
+        // groups SUM to 0 and multi-row groups skip the first row. See #1.
+        const n = toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
+        return Number.isNaN(n) ? 0 : n;
     },
     'COUNT'() {
         return 1;
@@ -505,17 +508,18 @@ const lineCreator: {
         return upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream);
     },
     'MIN'(upstream, dataIndex, dimInfo) {
-        return upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream);
+        return toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
     },
     'MAX'(upstream, dataIndex, dimInfo) {
-        return upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream);
+        return toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
     },
     'AVERAGE'(upstream, dataIndex, dimInfo, groupByDimInfo, groupByVal) {
         // FIXME: refactor, bad implementation.
         const collectLine = groupByDimInfo
             ? dimInfo.__collectionResult.mapByGroup[groupByVal + '']
             : dimInfo.__collectionResult.outList[0];
-        return (upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream) as number)
+        const n = toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
+        return (Number.isNaN(n) ? 0 : n)
             / collectLine[dimInfo.getCollectionInfo('COUNT').indexInLine];
     },
     // FIXME: refactor
@@ -541,8 +545,8 @@ const lineUpdater: {
     ) => OptionDataValue
 } = {
     'SUM'(val, upstream, dataIndex, dimInfo) {
-        // FIXME: handle other types
-        return (val as number) + (upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream) as number);
+        const n = toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
+        return Number.isNaN(n) ? (val as number) : (val as number) + n;
     },
     'COUNT'(val) {
         return (val as number) + 1;
@@ -551,18 +555,21 @@ const lineUpdater: {
         return val;
     },
     'MIN'(val, upstream, dataIndex, dimInfo) {
-        return Math.min(val as number, upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream) as number);
+        const n = toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
+        return Number.isNaN(n) ? (val as number) : Math.min(val as number, n);
     },
     'MAX'(val, upstream, dataIndex, dimInfo) {
-        return Math.max(val as number, upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream) as number);
+        const n = toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
+        return Number.isNaN(n) ? (val as number) : Math.max(val as number, n);
     },
     'AVERAGE'(val, upstream, dataIndex, dimInfo, groupByDimInfo, groupByVal) {
         // FIXME: refactor, bad implementation.
         const collectLine = groupByDimInfo
             ? dimInfo.__collectionResult.mapByGroup[groupByVal + '']
             : dimInfo.__collectionResult.outList[0];
+        const n = toNumber(upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream));
         return (val as number)
-            + (upstream.retrieveValue(dataIndex, dimInfo.indexInUpstream) as number)
+            + (Number.isNaN(n) ? 0 : n)
             / collectLine[dimInfo.getCollectionInfo('COUNT').indexInLine];
     },
     'Q1'(val, upstream, dataIndex, dimInfo) {

src/util.ts

@@ -27,6 +27,24 @@ export function hasOwn(own: object, prop: string): boolean {
     return own.hasOwnProperty(prop);
 }
 
+/**
+ * Coerce a dataset cell to a number for arithmetic aggregations.
+ * Empty / null / non-numeric values become NaN so callers can skip them
+ * instead of concatenating strings (e.g. '400' + '100' => '400100').
+ */
+export function toNumber(val: unknown): number {
+    if (val == null || val === '') {
+        return NaN;
+    }
+    if (typeof val === 'number') {
+        return val;
+    }
+    if (val instanceof Date) {
+        return +val;
+    }
+    return Number(val);
+}
+
 export function quantile(ascArr: number[], p: number): number {
     const H = (ascArr.length - 1) * p + 1;
     const h = Math.floor(H);