Merge pull request #2031 from plotly/aggregationInputIndicies

Add Aggregation user input _index

Author: bpostlethwaite

src/plots/plots.js

@@ -1039,9 +1039,21 @@ plots.supplyTransformDefaults = function(traceIn, traceOut, layout) {
             _module = transformsRegistry[type],
             transformOut;
 
+        /*
+         * Supply defaults may run twice. First pass runs all supply defaults steps
+         * and adds the _module to any output transforms.
+         * If transforms exist another pass is run so that any generated traces also
+         * go through supply defaults. This has the effect of rerunning
+         * supplyTransformDefaults. If the transform does not have a `transform`
+         * function it could not have generated any new traces and the second stage
+         * is unnecessary. We detect this case with the following variables.
+         */
+        var isFirstStage = !(transformIn._module && transformIn._module === _module),
+            doLaterStages = _module && typeof _module.transform === 'function';
+
         if(!_module) Lib.warn('Unrecognized transform type ' + type + '.');
 
-        if(_module && _module.supplyDefaults) {
+        if(_module && _module.supplyDefaults && (isFirstStage || doLaterStages)) {
             transformOut = _module.supplyDefaults(transformIn, traceOut, layout, traceIn);
             transformOut.type = type;
             transformOut._module = _module;

src/transforms/aggregate.js

@@ -166,31 +166,29 @@ exports.supplyDefaults = function(transformIn, traceOut) {
         arrayAttrs[groups] = 0;
     }
 
-    var aggregationsIn = transformIn.aggregations;
+    var aggregationsIn = transformIn.aggregations || [];
     var aggregationsOut = transformOut.aggregations = new Array(aggregationsIn.length);
     var aggregationOut;
 
     function coercei(attr, dflt) {
         return Lib.coerce(aggregationsIn[i], aggregationOut, aggAttrs, attr, dflt);
     }
 
-    if(aggregationsIn) {
-        for(i = 0; i < aggregationsIn.length; i++) {
-            aggregationOut = {};
-            var target = coercei('target');
-            var func = coercei('func');
-            var enabledi = coercei('enabled');
-
-            // add this aggregation to the output only if it's the first instance
-            // of a valid target attribute - or an unused target attribute with "count"
-            if(enabledi && target && (arrayAttrs[target] || (func === 'count' && arrayAttrs[target] === undefined))) {
-                if(func === 'stddev') coercei('funcmode');
-
-                arrayAttrs[target] = 0;
-                aggregationsOut[i] = aggregationOut;
-            }
-            else aggregationsOut[i] = {enabled: false};
+    for(i = 0; i < aggregationsIn.length; i++) {
+        aggregationOut = {_index: i};
+        var target = coercei('target');
+        var func = coercei('func');
+        var enabledi = coercei('enabled');
+
+        // add this aggregation to the output only if it's the first instance
+        // of a valid target attribute - or an unused target attribute with "count"
+        if(enabledi && target && (arrayAttrs[target] || (func === 'count' && arrayAttrs[target] === undefined))) {
+            if(func === 'stddev') coercei('funcmode');
+
+            arrayAttrs[target] = 0;
+            aggregationsOut[i] = aggregationOut;
         }
+        else aggregationsOut[i] = {enabled: false, _index: i};
     }
 
     // any array attributes we haven't yet covered, fill them with the default aggregation
@@ -199,7 +197,8 @@ exports.supplyDefaults = function(transformIn, traceOut) {
             aggregationsOut.push({
                 target: arrayAttrArray[i],
                 func: aggAttrs.func.dflt,
-                enabled: true
+                enabled: true,
+                _index: -1
             });
         }
     }

test/jasmine/tests/transform_aggregate_test.js

@@ -187,6 +187,26 @@ describe('aggregate', function() {
         expect(traceOut.marker.size).toEqual([10, 20]);
     });
 
+    // Regression test - throws before fix:
+    // https://github.com/plotly/plotly.js/issues/2024
+    it('can handle case where aggregation array is missing', function() {
+        Plotly.newPlot(gd, [{
+            x: [1, 2, 3, 4, 5],
+            y: [2, 4, 6, 8, 10],
+            marker: {size: [10, 10, 20, 20, 10]},
+            transforms: [{
+                type: 'aggregate',
+                groups: 'marker.size'
+            }]
+        }]);
+
+        var traceOut = gd._fullData[0];
+
+        expect(traceOut.x).toEqual([1, 3]);
+        expect(traceOut.y).toEqual([2, 6]);
+        expect(traceOut.marker.size).toEqual([10, 20]);
+    });
+
     it('handles median, mode, rms, & stddev for numeric data', function() {
         // again, nothing is going to barf with non-numeric data, but sometimes it
         // won't make much sense.
@@ -222,4 +242,40 @@ describe('aggregate', function() {
         expect(traceOut.marker.line.width).toBeCloseToArray([0.5, 0], 5);
         expect(traceOut.marker.color).toBeCloseToArray([Math.sqrt(1 / 3), 0], 5);
     });
+
+    it('links fullData aggregations to userData via _index', function() {
+        Plotly.newPlot(gd, [{
+            x: [1, 2, 3, 4, 5],
+            y: [2, 4, 6, 8, 10],
+            marker: {
+                size: [10, 10, 20, 20, 10],
+                color: ['red', 'green', 'blue', 'yellow', 'white']
+            },
+            transforms: [{
+                type: 'aggregate',
+                groups: 'marker.size',
+                aggregations: [
+                    {target: 'x', func: 'sum'},
+                    {target: 'x', func: 'avg'},
+                    {target: 'y', func: 'avg'},
+                ]
+            }]
+        }]);
+
+        var traceOut = gd._fullData[0];
+        var fullAggregation = traceOut.transforms[0];
+        var fullAggregations = fullAggregation.aggregations;
+        var enabledAggregations = fullAggregations.filter(function(agg) {
+            return agg.enabled;
+        });
+
+        expect(enabledAggregations[0].target).toEqual('x');
+        expect(enabledAggregations[0]._index).toEqual(0);
+
+        expect(enabledAggregations[1].target).toEqual('y');
+        expect(enabledAggregations[1]._index).toEqual(2);
+
+        expect(enabledAggregations[2].target).toEqual('marker.color');
+        expect(enabledAggregations[2]._index).toEqual(-1);
+    });
 });