Skip to content

Commit b436d52

Browse files
alexcjohnsonalexcjohnson
committed
ensure every trace defines _length in supplyDefaults, and abort transforms if it's falsy
1 parent e84d4b9 commit b436d52

26 files changed

+175
-75
lines changed

‎src/plots/plots.js

+5
Original file line numberDiff line numberDiff line change
@@ -1178,6 +1178,11 @@ plots.supplyTraceDefaults = function(traceIn, colorIndex, layout, traceInIndex)
11781178
};
11791179

11801180
plots.supplyTransformDefaults = function(traceIn, traceOut, layout) {
1181+
// For now we only allow transforms on 1D traces, ie those that specify a _length.
1182+
// If we were to implement 2D transforms, we'd need to have each transform
1183+
// describe its own applicability and disable itself when it doesn't apply.
1184+
if(!traceOut._length) return;
1185+
11811186
var globalTransforms = layout._globalTransforms || [];
11821187
var transformModules = layout._transformModules || [];
11831188

‎src/traces/box/calc.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,11 @@ module.exports = function calc(gd, trace) {
4848
var dPos = dv.minDiff / 2;
4949
var posBins = makeBins(posDistinct, dPos);
5050

51-
var vLen = val.length;
5251
var pLen = posDistinct.length;
5352
var ptsPerBin = initNestedArray(pLen);
5453

5554
// bin pts info per position bins
56-
for(i = 0; i < vLen; i++) {
55+
for(i = 0; i < trace._length; i++) {
5756
var v = val[i];
5857
if(!isNumeric(v)) continue;
5958

‎src/traces/box/defaults.js

+11-2
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,28 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
3838
function handleSampleDefaults(traceIn, traceOut, coerce, layout) {
3939
var y = coerce('y');
4040
var x = coerce('x');
41+
var hasX = x && x.length;
4142

42-
var defaultOrientation;
43+
var defaultOrientation, len;
4344

4445
if(y && y.length) {
4546
defaultOrientation = 'v';
46-
if(!x) coerce('x0');
47+
if(hasX) {
48+
len = Math.min(x.length, y.length);
49+
}
50+
else {
51+
coerce('x0');
52+
len = y.length;
53+
}
4754
} else if(x && x.length) {
4855
defaultOrientation = 'h';
4956
coerce('y0');
57+
len = x.length;
5058
} else {
5159
traceOut.visible = false;
5260
return;
5361
}
62+
traceOut._length = len;
5463

5564
var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults');
5665
handleCalendarDefaults(traceIn, traceOut, ['x', 'y'], layout);

‎src/traces/carpet/defaults.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,12 @@ module.exports = function supplyDefaults(traceIn, traceOut, dfltColor, fullLayou
4646
// corresponds to b and the second to a. This sounds backwards but ends up making sense
4747
// the important part to know is that when you write y[j][i], j goes from 0 to b.length - 1
4848
// and i goes from 0 to a.length - 1.
49-
var len = handleXYDefaults(traceIn, traceOut, coerce);
49+
var validData = handleXYDefaults(traceIn, traceOut, coerce);
50+
if(!validData) {
51+
traceOut.visible = false;
52+
}
5053

5154
if(traceOut._cheater) {
5255
coerce('cheaterslope');
5356
}
54-
55-
if(!len) {
56-
traceOut.visible = false;
57-
}
5857
};

‎src/traces/carpet/xy_defaults.js

+15-1
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,25 @@
99

1010
'use strict';
1111

12+
var is1D = require('../../lib').is1D;
13+
1214
module.exports = function handleXYDefaults(traceIn, traceOut, coerce) {
1315
var x = coerce('x');
16+
var hasX = x && x.length;
1417
var y = coerce('y');
18+
var hasY = y && y.length;
19+
if(!hasX && !hasY) return false;
1520

1621
traceOut._cheater = !x;
1722

18-
return !!x || !!y;
23+
if((!hasX || is1D(x)) && (!hasY || is1D(y))) {
24+
var len = hasX ? x.length : Infinity;
25+
if(hasY) len = Math.min(len, y.length);
26+
if(traceOut.a && traceOut.a.length) len = Math.min(len, traceOut.a.length);
27+
if(traceOut.b && traceOut.b.length) len = Math.min(len, traceOut.b.length);
28+
traceOut._length = len;
29+
}
30+
else traceOut._length = null;
31+
32+
return true;
1933
};

‎src/traces/choropleth/calc.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ var arraysToCalcdata = require('../scatter/arrays_to_calcdata');
1717
var calcSelection = require('../scatter/calc_selection');
1818

1919
module.exports = function calc(gd, trace) {
20-
var len = trace.locations.length;
20+
var len = trace._length;
2121
var calcTrace = new Array(len);
2222

2323
for(var i = 0; i < len; i++) {

‎src/traces/choropleth/defaults.js

+3-12
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,13 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
1919
}
2020

2121
var locations = coerce('locations');
22-
23-
var len;
24-
if(locations) len = locations.length;
25-
26-
if(!locations || !len) {
27-
traceOut.visible = false;
28-
return;
29-
}
30-
3122
var z = coerce('z');
32-
if(!Lib.isArrayOrTypedArray(z)) {
23+
24+
if(!(locations && locations.length && Lib.isArrayOrTypedArray(z) && z.length)) {
3325
traceOut.visible = false;
3426
return;
3527
}
36-
37-
if(z.length > len) traceOut.z = z.slice(0, len);
28+
traceOut._length = Math.min(locations.length, z.length);
3829

3930
coerce('locationmode');
4031

‎src/traces/contourcarpet/defaults.js

+1
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,6 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
6969
}
7070
} else {
7171
traceOut._defaultColor = defaultColor;
72+
traceOut._length = null;
7273
}
7374
};

‎src/traces/heatmap/convert_column_xyz.js

+7-15
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,16 @@ var Lib = require('../../lib');
1313
var BADNUM = require('../../constants/numerical').BADNUM;
1414

1515
module.exports = function convertColumnData(trace, ax1, ax2, var1Name, var2Name, arrayVarNames) {
16-
var col1 = trace[var1Name].slice(),
17-
col2 = trace[var2Name].slice(),
18-
textCol = trace.text,
19-
colLen = Math.min(col1.length, col2.length),
20-
hasColumnText = (textCol !== undefined && !Array.isArray(textCol[0])),
21-
col1Calendar = trace[var1Name + 'calendar'],
22-
col2Calendar = trace[var2Name + 'calendar'];
16+
var colLen = trace._length;
17+
var col1 = trace[var1Name].slice(0, colLen);
18+
var col2 = trace[var2Name].slice(0, colLen);
19+
var textCol = trace.text;
20+
var hasColumnText = (textCol !== undefined && Lib.is1D(textCol));
21+
var col1Calendar = trace[var1Name + 'calendar'];
22+
var col2Calendar = trace[var2Name + 'calendar'];
2323

2424
var i, j, arrayVar, newArray, arrayVarName;
2525

26-
for(i = 0; i < arrayVarNames.length; i++) {
27-
arrayVar = trace[arrayVarNames[i]];
28-
if(arrayVar) colLen = Math.min(colLen, arrayVar.length);
29-
}
30-
31-
if(colLen < col1.length) col1 = col1.slice(0, colLen);
32-
if(colLen < col2.length) col2 = col2.slice(0, colLen);
33-
3426
for(i = 0; i < colLen; i++) {
3527
col1[i] = ax1.d2c(col1[i], 0, col1Calendar);
3628
col2[i] = ax2.d2c(col2[i], 0, col2Calendar);

‎src/traces/heatmap/xyz_defaults.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ module.exports = function handleXYZDefaults(traceIn, traceOut, coerce, layout, x
2727
y = coerce(yName);
2828

2929
// column z must be accompanied by xName and yName arrays
30-
if(!x || !y) return 0;
30+
if(!(x && x.length && y && y.length)) return 0;
31+
32+
traceOut._length = Math.min(x.length, y.length, z.length);
3133
}
3234
else {
3335
x = coordDefaults(xName, coerce);
@@ -37,12 +39,14 @@ module.exports = function handleXYZDefaults(traceIn, traceOut, coerce, layout, x
3739
if(!isValidZ(z)) return 0;
3840

3941
coerce('transpose');
42+
43+
traceOut._length = null;
4044
}
4145

4246
var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults');
4347
handleCalendarDefaults(traceIn, traceOut, [xName, yName], layout);
4448

45-
return traceOut.z.length;
49+
return true;
4650
};
4751

4852
function coordDefaults(coordStr, coerce) {

‎src/traces/histogram/defaults.js

+15-8
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
2222
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
2323
}
2424

25-
var x = coerce('x'),
26-
y = coerce('y');
25+
var x = coerce('x');
26+
var y = coerce('y');
27+
var hasX = x && x.length;
28+
var hasY = y && y.length;
2729

2830
var cumulative = coerce('cumulative.enabled');
2931
if(cumulative) {
@@ -33,22 +35,27 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
3335

3436
coerce('text');
3537

36-
var orientation = coerce('orientation', (y && !x) ? 'h' : 'v'),
37-
sample = traceOut[orientation === 'v' ? 'x' : 'y'];
38+
var orientation = coerce('orientation', (hasY && !hasX) ? 'h' : 'v');
39+
var sampleLetter = orientation === 'v' ? 'x' : 'y';
40+
var aggLetter = orientation === 'v' ? 'y' : 'x';
41+
var sample = traceOut[sampleLetter];
3842

39-
if(!(sample && sample.length)) {
43+
var len = (hasX && hasY) ? Math.min(x.length && y.length) : (sample || []).length;
44+
45+
if(!len) {
4046
traceOut.visible = false;
4147
return;
4248
}
4349

50+
traceOut._length = len;
51+
4452
var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults');
4553
handleCalendarDefaults(traceIn, traceOut, ['x', 'y'], layout);
4654

47-
var hasAggregationData = traceOut[orientation === 'h' ? 'x' : 'y'];
55+
var hasAggregationData = traceOut[aggLetter];
4856
if(hasAggregationData) coerce('histfunc');
4957

50-
var binDirections = (orientation === 'h') ? ['y'] : ['x'];
51-
handleBinDefaults(traceIn, traceOut, coerce, binDirections);
58+
handleBinDefaults(traceIn, traceOut, coerce, [sampleLetter]);
5259

5360
handleStyleDefaults(traceIn, traceOut, coerce, defaultColor, layout);
5461

‎src/traces/histogram2d/calc.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ module.exports = function calc(gd, trace) {
3333

3434
var i, j, n, m;
3535

36-
var serieslen = Math.min(x.length, y.length);
36+
var serieslen = trace._length;
3737
if(x.length > serieslen) x.splice(serieslen, x.length - serieslen);
3838
if(y.length > serieslen) y.splice(serieslen, y.length - serieslen);
3939

‎src/traces/histogram2d/sample_defaults.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ var handleBinDefaults = require('../histogram/bin_defaults');
1414

1515

1616
module.exports = function handleSampleDefaults(traceIn, traceOut, coerce, layout) {
17-
var x = coerce('x'),
18-
y = coerce('y');
17+
var x = coerce('x');
18+
var y = coerce('y');
1919

2020
// we could try to accept x0 and dx, etc...
2121
// but that's a pretty weird use case.
@@ -25,6 +25,8 @@ module.exports = function handleSampleDefaults(traceIn, traceOut, coerce, layout
2525
return;
2626
}
2727

28+
traceOut._length = Math.min(x.length, y.length);
29+
2830
var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults');
2931
handleCalendarDefaults(traceIn, traceOut, ['x', 'y'], layout);
3032

‎src/traces/mesh3d/defaults.js

+3
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
8686
}
8787

8888
coerce('text');
89+
90+
// disable 1D transforms
91+
traceOut._length = null;
8992
};

‎src/traces/pie/defaults.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,28 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
1818
}
1919

2020
var coerceFont = Lib.coerceFont;
21+
var len;
2122

2223
var vals = coerce('values');
24+
var hasVals = Lib.isArrayOrTypedArray(vals) && vals.length;
2325
var labels = coerce('labels');
26+
if(Array.isArray(labels) && labels.length) {
27+
len = labels.length;
28+
if(hasVals) len = Math.min(len, vals.length);
29+
}
2430
if(!Array.isArray(labels)) {
25-
if(!Lib.isArrayOrTypedArray(vals) || !vals.length) {
31+
if(!hasVals) {
2632
// must have at least one of vals or labels
2733
traceOut.visible = false;
2834
return;
2935
}
3036

37+
len = vals.length;
38+
3139
coerce('label0');
3240
coerce('dlabel');
3341
}
42+
traceOut._length = len;
3443

3544
var lineWidth = coerce('marker.line.width');
3645
if(lineWidth) coerce('marker.line.color');

‎src/traces/pointcloud/defaults.js

+3
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor) {
4040
coerce('marker.sizemax');
4141
coerce('marker.border.color', defaultColor);
4242
coerce('marker.border.arearatio');
43+
44+
// disable 1D transforms - that would defeat the purpose of this trace type, performance!
45+
traceOut._length = null;
4346
};

‎src/traces/sankey/defaults.js

+4
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
6363
if(traceOut.node.label.some(missing)) {
6464
Lib.warn('Some of the nodes are neither sources nor targets, they will not be displayed.');
6565
}
66+
67+
// disable 1D transforms - arrays here are 1D but their lengths/meanings
68+
// don't match, between nodes and links
69+
traceOut._length = null;
6670
};

‎src/traces/scatter3d/defaults.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ function handleXYZDefaults(traceIn, traceOut, coerce, layout) {
7979
if(x && y && z) {
8080
// TODO: what happens if one is missing?
8181
len = Math.min(x.length, y.length, z.length);
82-
traceOut._xlength = traceOut._ylength = traceOut._zlength = len;
82+
traceOut._length = traceOut._xlength = traceOut._ylength = traceOut._zlength = len;
8383
}
8484

8585
return len;

‎src/traces/surface/defaults.js

+4
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
9696
colorscaleDefaults(
9797
traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'c'}
9898
);
99+
100+
// disable 1D transforms - currently surface does NOT support column data like heatmap does
101+
// you can use mesh3d for this use case, but not surface
102+
traceOut._length = null;
99103
};
100104

101105
function mapLegacy(traceIn, oldAttr, newAttr) {

‎src/traces/table/defaults.js

+3
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
5757
coerce('cells.line.color');
5858
coerce('cells.fill.color');
5959
Lib.coerceFont(coerce, 'cells.font', Lib.extendFlat({}, layout.font));
60+
61+
// disable 1D transforms
62+
traceOut._length = null;
6063
};

‎test/jasmine/tests/choropleth_test.js

+14-1
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,26 @@ describe('Test choropleth', function() {
3030
traceOut = {};
3131
});
3232

33-
it('should slice z if it is longer than locations', function() {
33+
it('should set _length based on locations and z but not slice', function() {
3434
traceIn = {
3535
locations: ['CAN', 'USA'],
3636
z: [1, 2, 3]
3737
};
3838

39+
Choropleth.supplyDefaults(traceIn, traceOut, defaultColor, layout);
40+
expect(traceOut.z).toEqual([1, 2, 3]);
41+
expect(traceOut.locations).toEqual(['CAN', 'USA']);
42+
expect(traceOut._length).toBe(2);
43+
44+
traceIn = {
45+
locations: ['CAN', 'USA', 'ALB'],
46+
z: [1, 2]
47+
};
48+
3949
Choropleth.supplyDefaults(traceIn, traceOut, defaultColor, layout);
4050
expect(traceOut.z).toEqual([1, 2]);
51+
expect(traceOut.locations).toEqual(['CAN', 'USA', 'ALB']);
52+
expect(traceOut._length).toBe(2);
4153
});
4254

4355
it('should make trace invisible if locations is not defined', function() {
@@ -51,6 +63,7 @@ describe('Test choropleth', function() {
5163

5264
it('should make trace invisible if z is not an array', function() {
5365
traceIn = {
66+
locations: ['CAN', 'USA'],
5467
z: 'no gonna work'
5568
};
5669

0 commit comments

Comments
 (0)