Make transforms apply recursively

Author: rreusser

src/plots/plots.js

@@ -1053,25 +1053,42 @@ plots.supplyTransformDefaults = function(traceIn, traceOut, layout) {
 };
 
 function applyTransforms(fullTrace, fullData, layout, fullLayout) {
-    var container = fullTrace.transforms,
-        dataOut = [fullTrace];
+    var dataOut = [];
 
-    for(var i = 0; i < container.length; i++) {
-        var transform = container[i],
-            _module = transformsRegistry[transform.type];
+    // Takes single trace, returns list of expanded traces
+    function recurse(trace, transformIndex) {
+        var transformedTraces = [trace];
+        var transform = trace.transforms[transformIndex];
+        var _module = transformsRegistry[transform.type];
 
         if(_module && _module.transform) {
-            dataOut = _module.transform(dataOut, {
+            transformedTraces = _module.transform([trace], {
                 transform: transform,
-                fullTrace: fullTrace,
+                fullTrace: trace,
                 fullData: fullData,
                 layout: layout,
                 fullLayout: fullLayout,
-                transformIndex: i
+                transformIndex: transformIndex
             });
         }
+
+        for(var i = 0; i < transformedTraces.length; i++) {
+            if(transformIndex >= transformedTraces[i].transforms.length - 1) {
+                // If we get here, we've reached the end of the line and this expanded
+                // trace has no more transforms to apply. That is, it's an output trace
+                // so we push it onto dataOut.
+                dataOut.push(transformedTraces[i]);
+            } else {
+                // Otherwise there are more transforms to be performed. We don't need
+                // to worry about the return value since every transform will eventually
+                // reach the end of the line and push its result onto the output.
+                recurse(transformedTraces[i], transformIndex + 1);
+            }
+        }
     }
 
+    recurse(fullTrace, 0);
+
     return dataOut;
 }
 

src/transforms/groupby.js

@@ -137,8 +137,22 @@ function pasteArray(newTrace, trace, j, a) {
     );
 }
 
+// In order for groups to apply correctly to other transform data (e.g.
+// a filter transform), we have to break the connection and clone the
+// transforms so that each group writes grouped values into a different
+// destination. This function does not break the array reference
+// connection between the split transforms it creates. That's handled
+// ininialize, which creates a new empty array for each arrayAttr.
+function cloneTransforms(newTrace) {
+    var transforms = newTrace.transforms;
+    newTrace.transforms = [];
+    for(var j = 0; j < transforms.length; j++) {
+        newTrace.transforms[j] = Lib.extendDeepNoArrays({}, transforms[j]);
+    }
+}
+
 function transformOne(trace, state) {
-    var i;
+    var i, j;
     var opts = state.transform;
     var groups = trace.transforms[state.transformIndex].groups;
 
@@ -163,12 +177,14 @@ function transformOne(trace, state) {
 
         var newTrace = newData[i] = Lib.extendDeepNoArrays({}, trace);
 
+        cloneTransforms(newTrace);
+
         arrayAttrs.forEach(initializeArray.bind(null, newTrace));
 
-        for(var j = 0; j < len; j++) {
+        for(j = 0; j < len; j++) {
             if(groups[j] !== groupName) continue;
 
-            arrayAttrs.forEach(pasteArray.bind(0, newTrace, trace, j));
+            arrayAttrs.forEach(pasteArray.bind(null, newTrace, trace, j));
         }
 
         newTrace.name = groupName;
@@ -180,5 +196,12 @@ function transformOne(trace, state) {
         newTrace = Lib.extendDeepNoArrays(newTrace, styleLookup[groupName] || {});
     }
 
+    for(i = 0; i < newData.length; i++) {
+        var data = newData[i];
+        var transforms = data.transforms.slice();
+        transforms.splice(state.transformIndex, 1);
+        data.transforms = transforms;
+    }
+
     return newData;
 }

test/jasmine/tests/transform_multi_test.js

@@ -728,7 +728,7 @@ describe('restyle applied on transforms:', function() {
 
 });
 
-describe('supplyDefaults with groupby + filter', function () {
+describe('supplyDefaults with groupby + filter', function() {
     function calcDatatoTrace(calcTrace) {
         return calcTrace[0].trace;
     }
@@ -745,16 +745,16 @@ describe('supplyDefaults with groupby + filter', function () {
         return gd.calcdata.map(calcDatatoTrace);
     }
 
-    it('computes the correct data when filter target blank', function () {
+    it('computes the correct data when filter target blank', function() {
         var out = _transform([{
             x: [1, 2, 3, 4, 5, 6, 7],
             y: [4, 6, 5, 7, 6, 8, 9],
             transforms: [{
-                type: "filter",
-                operation: "<",
+                type: 'filter',
+                operation: '<',
                 value: 6.5
             }, {
-                type: "groupby",
+                type: 'groupby',
                 groups: [1, 1, 1, 2, 2, 2, 2]
             }]
         }]);
@@ -766,18 +766,46 @@ describe('supplyDefaults with groupby + filter', function () {
         expect(out[1].y).toEqual([7, 6, 8]);
     });
 
-    it('computes the correct data when filter target present', function () {
+    it('computes the correct data when fiter + groupby', function() {
         var out = _transform([{
+            x: [5, 4, 3],
+            y: [6, 5, 4],
+        }, {
             x: [1, 2, 3, 4, 5, 6, 7],
-            y: [4, 6, 5, 7, 6, 8, 9],
+            y: [4, 6, 5, 7, 8, 9, 10],
             transforms: [{
-                type: "filter",
+                type: 'filter',
                 target: [1, 2, 3, 4, 5, 6, 7],
-                operation: "<",
+                operation: '<',
                 value: 6.5
             }, {
-                type: "groupby",
+                type: 'groupby',
+                groups: [1, 1, 1, 2, 2, 2, 2]
+            }]
+        }]);
+
+        expect(out[0].x).toEqual([5, 4, 3]);
+        expect(out[0].y).toEqual([6, 5, 4]);
+
+        expect(out[1].x).toEqual([1, 2, 3]);
+        expect(out[1].y).toEqual([4, 6, 5]);
+
+        expect(out[2].x).toEqual([4, 5, 6]);
+        expect(out[2].y).toEqual([7, 8, 9]);
+    });
+
+    it('computes the correct data when groupby + filter', function() {
+        var out = _transform([{
+            x: [1, 2, 3, 4, 5, 6, 7],
+            y: [4, 6, 5, 7, 6, 8, 9],
+            transforms: [{
+                type: 'groupby',
                 groups: [1, 1, 1, 2, 2, 2, 2]
+            }, {
+                type: 'filter',
+                target: [1, 2, 3, 4, 5, 6, 7],
+                operation: '<',
+                value: 6.5
             }]
         }]);