stop overwriting fullTrace arrays in convert_column_xyz

alexcjohnson · alexcjohnson · commit 6b201a15985d · 2018-04-04T15:48:35.000-04:00
diff --git a/src/plots/plots.js b/src/plots/plots.js
@@ -2364,14 +2364,15 @@ plots.doCalcdata = function(gd, traces) {
     // clear stuff that should recomputed in 'regular' loop
     if(hasCalcTransform) clearAxesCalc(axList);
 
-    // 'regular' loop
-    for(i = 0; i < fullData.length; i++) {
-        var cd = [];
-
+    function calci(i, isContainer) {
         trace = fullData[i];
+        _module = trace._module;
+
+        if(!!_module.isContainer !== isContainer) return;
+
+        var cd = [];
 
         if(trace.visible === true) {
-            _module = trace._module;
 
             // keep ref of index-to-points map object of the *last* enabled transform,
             // this index-to-points map object is required to determine the calcdata indices
@@ -2406,6 +2407,11 @@ plots.doCalcdata = function(gd, traces) {
         calcdata[i] = cd;
     }
 
+    // 'regular' loop - make sure container traces (eg carpet) calc before
+    // contained traces (eg contourcarpet)
+    for(i = 0; i < fullData.length; i++) calci(i, true);
+    for(i = 0; i < fullData.length; i++) calci(i, false);
+
     Registry.getComponentMethod('fx', 'calc')(gd);
 };
 
diff --git a/src/traces/carpet/axis_defaults.js b/src/traces/carpet/axis_defaults.js
@@ -103,7 +103,10 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options)
         handleCalendarDefaults(containerIn, containerOut, 'calendar', options.calendar);
     }
 
+    // we need some of the other functions setConvert attaches, but for
+    // path finding, override pixel scaling to simple passthrough (identity)
     setConvert(containerOut, options.fullLayout);
+    containerOut.c2p = Lib.identity;
 
     var dfltColor = coerce('color', options.dfltColor);
     // if axis.color was provided, use it for fonts too; otherwise,
diff --git a/src/traces/carpet/calc.js b/src/traces/carpet/calc.js
@@ -17,25 +17,37 @@ var calcLabels = require('./calc_labels');
 var calcClipPath = require('./calc_clippath');
 var clean2dArray = require('../heatmap/clean_2d_array');
 var smoothFill2dArray = require('./smooth_fill_2d_array');
+var hasColumns = require('./has_columns');
+var convertColumnData = require('../heatmap/convert_column_xyz');
+var setConvert = require('./set_convert');
 
 module.exports = function calc(gd, trace) {
-    var xa = Axes.getFromId(gd, trace.xaxis || 'x');
-    var ya = Axes.getFromId(gd, trace.yaxis || 'y');
+    var xa = Axes.getFromId(gd, trace.xaxis);
+    var ya = Axes.getFromId(gd, trace.yaxis);
     var aax = trace.aaxis;
     var bax = trace.baxis;
-    var a = trace._a = trace.a;
-    var b = trace._b = trace.b;
 
-    var t = {};
-    var x;
+    var x = trace.x;
     var y = trace.y;
+    var cols = [];
+    if(x && !hasColumns(x)) cols.push('x');
+    if(y && !hasColumns(y)) cols.push('y');
+
+    if(cols.length) {
+        convertColumnData(trace, aax, bax, 'a', 'b', cols);
+    }
+
+    var a = trace._a = trace._a || trace.a;
+    var b = trace._b = trace._b || trace.b;
+    x = trace._x || trace.x;
+    y = trace._y || trace.y;
+
+    var t = {};
 
     if(trace._cheater) {
         var avals = aax.cheatertype === 'index' ? a.length : a;
         var bvals = bax.cheatertype === 'index' ? b.length : b;
         x = cheaterBasis(avals, bvals, trace.cheaterslope);
-    } else {
-        x = trace.x;
     }
 
     trace._x = x = clean2dArray(x);
@@ -48,6 +60,8 @@ module.exports = function calc(gd, trace) {
     smoothFill2dArray(x, a, b);
     smoothFill2dArray(y, a, b);
 
+    setConvert(trace);
+
     // create conversion functions that depend on the data
     trace.setScale();
 
diff --git a/src/traces/carpet/calc_gridlines.js b/src/traces/carpet/calc_gridlines.js
@@ -16,14 +16,14 @@ module.exports = function calcGridlines(trace, cd, axisLetter, crossAxisLetter)
     var eps, bounds, n1, n2, n, value, v;
     var j1, v0, v1, d;
 
-    var data = trace[axisLetter];
+    var data = trace['_' + axisLetter];
     var axis = trace[axisLetter + 'axis'];
 
     var gridlines = axis._gridlines = [];
     var minorgridlines = axis._minorgridlines = [];
     var boundarylines = axis._boundarylines = [];
 
-    var crossData = trace[crossAxisLetter];
+    var crossData = trace['_' + crossAxisLetter];
     var crossAxis = trace[crossAxisLetter + 'axis'];
 
     if(axis.tickmode === 'array') {
@@ -34,8 +34,8 @@ module.exports = function calcGridlines(trace, cd, axisLetter, crossAxisLetter)
     var ycp = trace._yctrl;
     var nea = xcp[0].length;
     var neb = xcp.length;
-    var na = trace.a.length;
-    var nb = trace.b.length;
+    var na = trace._a.length;
+    var nb = trace._b.length;
 
     Axes.prepTicks(axis);
 
diff --git a/src/traces/carpet/defaults.js b/src/traces/carpet/defaults.js
@@ -12,7 +12,6 @@
 var Lib = require('../../lib');
 var handleXYDefaults = require('./xy_defaults');
 var handleABDefaults = require('./ab_defaults');
-var setConvert = require('./set_convert');
 var attributes = require('./attributes');
 var colorAttrs = require('../../components/color/attributes');
 
@@ -49,8 +48,6 @@ module.exports = function supplyDefaults(traceIn, traceOut, dfltColor, fullLayou
     // and i goes from 0 to a.length - 1.
     var len = handleXYDefaults(traceIn, traceOut, coerce);
 
-    setConvert(traceOut);
-
     if(traceOut._cheater) {
         coerce('cheaterslope');
     }
diff --git a/src/traces/carpet/index.js b/src/traces/carpet/index.js
@@ -16,6 +16,7 @@ Carpet.supplyDefaults = require('./defaults');
 Carpet.plot = require('./plot');
 Carpet.calc = require('./calc');
 Carpet.animatable = true;
+Carpet.isContainer = true; // so carpet traces get `calc` before other traces
 
 Carpet.moduleType = 'trace';
 Carpet.name = 'carpet';
diff --git a/src/traces/carpet/set_convert.js b/src/traces/carpet/set_convert.js
@@ -25,10 +25,10 @@ var createJDerivativeEvaluator = require('./create_j_derivative_evaluator');
  *   p: screen-space pixel coordinates
  */
 module.exports = function setConvert(trace) {
-    var a = trace.a;
-    var b = trace.b;
-    var na = trace.a.length;
-    var nb = trace.b.length;
+    var a = trace._a;
+    var b = trace._b;
+    var na = a.length;
+    var nb = b.length;
     var aax = trace.aaxis;
     var bax = trace.baxis;
 
@@ -60,10 +60,6 @@ module.exports = function setConvert(trace) {
         return a < amin || a > amax || b < bmin || b > bmax;
     };
 
-    // XXX: ONLY PASSTHRU. ONLY. No, ONLY.
-    aax.c2p = function(v) { return v; };
-    bax.c2p = function(v) { return v; };
-
     trace.setScale = function() {
         var x = trace._x;
         var y = trace._y;
diff --git a/src/traces/carpet/xy_defaults.js b/src/traces/carpet/xy_defaults.js
@@ -9,28 +9,11 @@
 
 'use strict';
 
-var hasColumns = require('./has_columns');
-var convertColumnData = require('../heatmap/convert_column_xyz');
-
 module.exports = function handleXYDefaults(traceIn, traceOut, coerce) {
-    var cols = [];
     var x = coerce('x');
-
-    var needsXTransform = x && !hasColumns(x);
-    if(needsXTransform) cols.push('x');
-
-    traceOut._cheater = !x;
-
     var y = coerce('y');
 
-    var needsYTransform = y && !hasColumns(y);
-    if(needsYTransform) cols.push('y');
-
-    if(!x && !y) return;
-
-    if(cols.length) {
-        convertColumnData(traceOut, traceOut.aaxis, traceOut.baxis, 'a', 'b', cols);
-    }
+    traceOut._cheater = !x;
 
-    return true;
+    return !!x || !!y;
 };
diff --git a/src/traces/contourcarpet/calc.js b/src/traces/contourcarpet/calc.js
@@ -70,15 +70,17 @@ function heatmappishCalc(gd, trace) {
     bax._minDtick = 0;
 
     if(hasColumns(trace)) convertColumnData(trace, aax, bax, 'a', 'b', ['z']);
+    a = trace._a = trace._a || trace.a;
+    b = trace._b = trace._b || trace.b;
 
-    a = trace.a ? aax.makeCalcdata(trace, 'a') : [];
-    b = trace.b ? bax.makeCalcdata(trace, 'b') : [];
+    a = a ? aax.makeCalcdata(trace, '_a') : [];
+    b = b ? bax.makeCalcdata(trace, '_b') : [];
     a0 = trace.a0 || 0;
     da = trace.da || 1;
     b0 = trace.b0 || 0;
     db = trace.db || 1;
 
-    z = clean2dArray(trace.z, trace.transpose);
+    z = trace._z = clean2dArray(trace._z || trace.z, trace.transpose);
 
     trace._emptypoints = findEmpties(z);
     trace._interpz = interp2d(z, trace._emptypoints, trace._interpz);
diff --git a/src/traces/heatmap/calc.js b/src/traces/heatmap/calc.js
@@ -58,10 +58,12 @@ module.exports = function calc(gd, trace) {
         z = binned.z;
     }
     else {
+        var zIn = trace.z;
         if(hasColumns(trace)) {
             convertColumnData(trace, xa, ya, 'x', 'y', ['z']);
-            x = trace.x;
-            y = trace.y;
+            x = trace._x;
+            y = trace._y;
+            zIn = trace._z;
         } else {
             x = trace.x ? xa.makeCalcdata(trace, 'x') : [];
             y = trace.y ? ya.makeCalcdata(trace, 'y') : [];
@@ -72,7 +74,7 @@ module.exports = function calc(gd, trace) {
         y0 = trace.y0 || 0;
         dy = trace.dy || 1;
 
-        z = clean2dArray(trace.z, trace.transpose);
+        z = clean2dArray(zIn, trace.transpose);
 
         if(isContour || trace.connectgaps) {
             trace._emptypoints = findEmpties(z);
@@ -131,7 +133,7 @@ module.exports = function calc(gd, trace) {
         x: xArray,
         y: yArray,
         z: z,
-        text: trace.text
+        text: trace._text || trace.text
     };
 
     if(xIn && xIn.length === xArray.length - 1) cd0.xCenter = xIn;
diff --git a/src/traces/heatmap/convert_column_xyz.js b/src/traces/heatmap/convert_column_xyz.js
@@ -66,14 +66,10 @@ module.exports = function convertColumnData(trace, ax1, ax2, var1Name, var2Name,
         }
     }
 
-    // hack for Plotly.react - save the input arrays for diffing purposes
-    trace['_input_' + var1Name] = trace[var1Name];
-    trace['_input_' + var2Name] = trace[var2Name];
-    trace[var1Name] = col1vals;
-    trace[var2Name] = col2vals;
+    trace['_' + var1Name] = col1vals;
+    trace['_' + var2Name] = col2vals;
     for(j = 0; j < arrayVarNames.length; j++) {
-        trace['_input_' + arrayVarNames[j]] = trace[arrayVarNames[j]];
-        trace[arrayVarNames[j]] = newArrays[j];
+        trace['_' + arrayVarNames[j]] = newArrays[j];
     }
-    if(hasColumnText) trace.text = text;
+    if(hasColumnText) trace._text = text;
 };
diff --git a/test/jasmine/tests/heatmap_test.js b/test/jasmine/tests/heatmap_test.js
@@ -174,17 +174,21 @@ describe('heatmap convertColumnXYZ', function() {
     var xa = makeMockAxis();
     var ya = makeMockAxis();
 
+    function checkConverted(trace, x, y, z) {
+        convertColumnXYZ(trace, xa, ya, 'x', 'y', ['z']);
+        expect(trace._x).toEqual(x);
+        expect(trace._y).toEqual(y);
+        expect(trace._z).toEqual(z);
+    }
+
     it('should convert x/y/z columns to z(x,y)', function() {
         trace = {
             x: [1, 1, 1, 2, 2, 2],
             y: [1, 2, 3, 1, 2, 3],
             z: [1, 2, 3, 4, 5, 6]
         };
 
-        convertColumnXYZ(trace, xa, ya, 'x', 'y', ['z']);
-        expect(trace.x).toEqual([1, 2]);
-        expect(trace.y).toEqual([1, 2, 3]);
-        expect(trace.z).toEqual([[1, 4], [2, 5], [3, 6]]);
+        checkConverted(trace, [1, 2], [1, 2, 3], [[1, 4], [2, 5], [3, 6]]);
     });
 
     it('should convert x/y/z columns to z(x,y) with uneven dimensions', function() {
@@ -194,10 +198,7 @@ describe('heatmap convertColumnXYZ', function() {
             z: [1, 2, 4, 5, 6]
         };
 
-        convertColumnXYZ(trace, xa, ya, 'x', 'y', ['z']);
-        expect(trace.x).toEqual([1, 2]);
-        expect(trace.y).toEqual([1, 2, 3]);
-        expect(trace.z).toEqual([[1, 4], [2, 5], [, 6]]);
+        checkConverted(trace, [1, 2], [1, 2, 3], [[1, 4], [2, 5], [, 6]]);
     });
 
     it('should convert x/y/z columns to z(x,y) with missing values', function() {
@@ -207,10 +208,7 @@ describe('heatmap convertColumnXYZ', function() {
             z: [1, null, 4, 5, 6]
         };
 
-        convertColumnXYZ(trace, xa, ya, 'x', 'y', ['z']);
-        expect(trace.x).toEqual([1, 2]);
-        expect(trace.y).toEqual([1, 2, 3]);
-        expect(trace.z).toEqual([[1, 4], [null, 5], [, 6]]);
+        checkConverted(trace, [1, 2], [1, 2, 3], [[1, 4], [null, 5], [, 6]]);
     });
 
     it('should convert x/y/z/text columns to z(x,y) and text(x,y)', function() {
@@ -222,7 +220,7 @@ describe('heatmap convertColumnXYZ', function() {
         };
 
         convertColumnXYZ(trace, xa, ya, 'x', 'y', ['z']);
-        expect(trace.text).toEqual([['a', 'd'], ['b', 'e'], ['c', 'f']]);
+        expect(trace._text).toEqual([['a', 'd'], ['b', 'e'], ['c', 'f']]);
     });
 
     it('should convert x/y/z columns to z(x,y) with out-of-order data', function() {
@@ -253,20 +251,19 @@ describe('heatmap convertColumnXYZ', function() {
             ]
         };
 
-        convertColumnXYZ(trace, xa, ya, 'x', 'y', ['z']);
-        expect(trace.x).toEqual(
-            [-88596, -65484, -42372, -19260, 3852, 26964, 50076, 73188]);
-        expect(trace.y).toEqual(
-            [-78096.2, -52106.6, -26117, -127.4, 25862.2, 51851.8, 77841.4]);
-        expect(trace.z).toEqual([
-            [,, 4.154291, 4.404264, 4.33847, 4.270931,,, ],
-            [, 4.339848, 4.39907, 4.345006, 4.315032, 4.295618, 4.262052,, ],
-            [3.908434, 4.433257, 4.364234, 4.308714, 4.275516, 4.126979, 4.296483, 4.320471],
-            [4.032226, 4.381492, 4.328922, 4.24046, 4.349151, 4.202861, 4.256402, 4.28972],
-            [3.956225, 4.337909, 4.31226, 4.259435, 4.146854, 4.235799, 4.238752, 4.299876],
-            [, 4.210373, 4.32009, 4.246728, 4.293992, 4.316364, 4.361856,, ],
-            [,, 4.234497, 4.321701, 4.450315, 4.416136,,, ]
-        ]);
+        checkConverted(trace,
+            [-88596, -65484, -42372, -19260, 3852, 26964, 50076, 73188],
+            [-78096.2, -52106.6, -26117, -127.4, 25862.2, 51851.8, 77841.4],
+            [
+                [,, 4.154291, 4.404264, 4.33847, 4.270931,,, ],
+                [, 4.339848, 4.39907, 4.345006, 4.315032, 4.295618, 4.262052,, ],
+                [3.908434, 4.433257, 4.364234, 4.308714, 4.275516, 4.126979, 4.296483, 4.320471],
+                [4.032226, 4.381492, 4.328922, 4.24046, 4.349151, 4.202861, 4.256402, 4.28972],
+                [3.956225, 4.337909, 4.31226, 4.259435, 4.146854, 4.235799, 4.238752, 4.299876],
+                [, 4.210373, 4.32009, 4.246728, 4.293992, 4.316364, 4.361856,, ],
+                [,, 4.234497, 4.321701, 4.450315, 4.416136,,, ]
+            ]
+        );
     });
 
     it('should convert x/y/z columns with nulls to z(x,y)', function() {
@@ -282,11 +279,7 @@ describe('heatmap convertColumnXYZ', function() {
             z: [0, 50, 100, 50, null, 255, 100, 510, 1010]
         };
 
-        convertColumnXYZ(trace, xa, ya, 'x', 'y', ['z']);
-
-        expect(trace.x).toEqual([0, 5, 10]);
-        expect(trace.y).toEqual([0, 5, 10]);
-        expect(trace.z).toEqual([
+        checkConverted(trace, [0, 5, 10], [0, 5, 10], [
             [0, 50, 100],
             [50, undefined, 510],
             [100, 255, 1010]
diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js

Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,10 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options)`
`103`	`103`	`handleCalendarDefaults(containerIn, containerOut, 'calendar', options.calendar);`
`104`	`104`	`}`
`105`	`105`
	`106`	`+ // we need some of the other functions setConvert attaches, but for`
	`107`	`+ // path finding, override pixel scaling to simple passthrough (identity)`
`106`	`108`	`setConvert(containerOut, options.fullLayout);`
	`109`	`+ containerOut.c2p = Lib.identity;`
`107`	`110`
`108`	`111`	`var dfltColor = coerce('color', options.dfltColor);`
`109`	`112`	`// if axis.color was provided, use it for fonts too; otherwise,`