Call clearTraceDefaultColors from groupby

Author: rreusser

src/plots/plots.js

@@ -664,31 +664,38 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa
 // supplyDefault'd and inherited as *colors* instead of an actual null
 // attribute which needs to be supplydefaulted by the individual
 // expanded traces.
-plots.clearExpandedTraceDefaultColors = function(expandedTraces) {
-    var colorAttrs, path, trace, i, j;
+plots.clearTraceDefaultColors = function(trace) {
+    var colorAttrs, path, i;
 
-    // A better check *might* be to explicitly check for a groupby transform
-    if(expandedTraces.length <= 1) return;
-
-    function locateExpandedTraceAttrs(attr, attrName, attrs, level) {
+    // This uses weird closure state in order to satisfy the linter rule
+    // that we can't create functions in a loop.
+    function locateColorAttrs(attr, attrName, attrs, level) {
         path[level] = attrName;
         path.length = level + 1;
         if(attr.useExpandedTraceDefaultColor) {
             colorAttrs.push(path.join('.'));
         }
     }
 
-    for(i = 0; i < expandedTraces.length; i++) {
-        trace = expandedTraces[i];
-        colorAttrs = [];
-        path = [];
+    path = [];
 
-        if(!trace || !trace._module) continue;
+    // Get the cached colorAttrs:
+    colorAttrs = trace._module._colorAttrs;
 
-        PlotSchema.crawl(trace._module.attributes, locateExpandedTraceAttrs);
+    // Or else compute and cache the colorAttrs on the module:
+    if(!colorAttrs) {
+        trace._module._colorAttrs = colorAttrs = [];
+        PlotSchema.crawl(
+            trace._module.attributes,
+            locateColorAttrs
+        );
+    }
 
-        for(j = 0; j < colorAttrs.length; j++) {
-            Lib.nestedProperty(trace, colorAttrs[j]).set(null);
+    for(i = 0; i < colorAttrs.length; i++) {
+        var origprop = Lib.nestedProperty(trace, '_input.' + colorAttrs[i]);
+
+        if(!origprop.get()) {
+            Lib.nestedProperty(trace, colorAttrs[i]).set(null);
         }
     }
 };
@@ -728,11 +735,9 @@ plots.supplyDataDefaults = function(dataIn, dataOut, layout, fullLayout) {
         if(fullTrace.transforms && fullTrace.transforms.length) {
             var expandedTraces = applyTransforms(fullTrace, dataOut, layout, fullLayout);
 
-            plots.clearExpandedTraceDefaultColors(expandedTraces);
-
             for(var j = 0; j < expandedTraces.length; j++) {
-                var expandedTrace = expandedTraces[j],
-                    fullExpandedTrace = plots.supplyTraceDefaults(expandedTrace, cnt, fullLayout, i);
+                var expandedTrace = expandedTraces[j];
+                var fullExpandedTrace = plots.supplyTraceDefaults(expandedTrace, cnt, fullLayout, i);
 
                 // mutate uid here using parent uid and expanded index
                 // to promote consistency between update calls

src/transforms/groupby.js

@@ -10,6 +10,7 @@
 
 var Lib = require('../lib');
 var PlotSchema = require('../plot_api/plot_schema');
+var Plots = require('../plots/plots');
 
 exports.moduleType = 'transform';
 
@@ -172,6 +173,8 @@ function transformOne(trace, state) {
 
         newTrace.name = groupName;
 
+        Plots.clearTraceDefaultColors(newTrace);
+
         // there's no need to coerce styleLookup[groupName] here
         // as another round of supplyDefaults is done on the transformed traces
         newTrace = Lib.extendDeepNoArrays(newTrace, styleLookup[groupName] || {});

test/jasmine/tests/plotschema_test.js

@@ -76,7 +76,7 @@ describe('plot schema', function() {
                     var valObject = valObjects[attr.valType],
                         opts = valObject.requiredOpts
                             .concat(valObject.otherOpts)
-                            .concat(['valType', 'description', 'role', 'editType']);
+                            .concat(['valType', 'description', 'role', 'editType', 'useExpandedTraceDefaultColor']);
 
                     Object.keys(attr).forEach(function(key) {
                         expect(opts.indexOf(key) !== -1).toBe(true, key, attr);

test/jasmine/tests/transform_groupby_test.js

@@ -1,4 +1,5 @@
 var Plotly = require('@lib/index');
+var Plots = require('@src/plots/plots');
 var Lib = require('@src/lib');
 
 var createGraphDiv = require('../assets/create_graph_div');
@@ -236,9 +237,38 @@ describe('groupby', function() {
                 done();
             });
         });
+    });
+
+    describe('many-to-many transforms', function() {
+        it('varies the color for each expanded trace', function() {
+            var uniqueColors = {};
+            var dataOut = [];
+            var dataIn = [{
+                y: [1, 2, 3],
+                transforms: [
+                    {type: 'filter', operation: '<', value: 4},
+                    {type: 'groupby', groups: ['a', 'b', 'c']}
+                ]
+            }, {
+                y: [4, 5, 6],
+                transforms: [
+                    {type: 'filter', operation: '<', value: 4},
+                    {type: 'groupby', groups: ['a', 'b', 'b']}
+                ]
+            }];
 
+            Plots.supplyDataDefaults(dataIn, dataOut, {}, {});
+
+            for(var i = 0; i < dataOut.length; i++) {
+                uniqueColors[dataOut[i].marker.color] = true;
+            }
+
+            // Confirm that five total colors exist:
+            expect(Object.keys(uniqueColors).length).toEqual(5);
+        });
     });
 
+
     // these tests can be shortened, once the meaning of edge cases gets clarified
     describe('symmetry/degeneracy testing of one-to-many transforms on arbitrary arrays where there is no grouping (implicit 1):', function() {
         'use strict';