diff --git a/src/plots/plots.js b/src/plots/plots.js index e8ff7e50bc8..e27a7710fbc 100644 --- a/src/plots/plots.js +++ b/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; diff --git a/src/transforms/aggregate.js b/src/transforms/aggregate.js index 932e54f4ccc..3000404c080 100644 --- a/src/transforms/aggregate.js +++ b/src/transforms/aggregate.js @@ -155,7 +155,7 @@ 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; @@ -163,23 +163,21 @@ exports.supplyDefaults = function(transformIn, traceOut) { 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 @@ -188,7 +186,8 @@ exports.supplyDefaults = function(transformIn, traceOut) { aggregationsOut.push({ target: arrayAttrArray[i], func: aggAttrs.func.dflt, - enabled: true + enabled: true, + _index: -1 }); } } diff --git a/test/jasmine/tests/transform_aggregate_test.js b/test/jasmine/tests/transform_aggregate_test.js index 0377963138b..5da7dfaa063 100644 --- a/test/jasmine/tests/transform_aggregate_test.js +++ b/test/jasmine/tests/transform_aggregate_test.js @@ -190,6 +190,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. @@ -225,4 +245,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); + }); });