Skip to content

Commit 96fe262

Browse files
archmojarchmoj
authored
Merge pull request #4063 from plotly/reuse-merge-array-cast-positive
Filter numbers during calc step
2 parents 539d7b7 + f9db523 commit 96fe262

File tree

7 files changed

+93
-13
lines changed

7 files changed

+93
-13
lines changed

‎src/lib/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ lib.mergeArray = function(traceAttr, cd, cdAttr, fn) {
474474
lib.mergeArrayCastPositive = function(traceAttr, cd, cdAttr) {
475475
return lib.mergeArray(traceAttr, cd, cdAttr, function(v) {
476476
var w = +v;
477-
return w > 0 ? w : 0;
477+
return !isFinite(w) ? 0 : w > 0 ? w : 0;
478478
});
479479
};
480480

‎src/traces/funnel/arrays_to_calcdata.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,24 @@
88

99
'use strict';
1010

11-
var mergeArray = require('../../lib').mergeArray;
11+
var Lib = require('../../lib');
1212

1313
// arrayOk attributes, merge them into calcdata array
1414
module.exports = function arraysToCalcdata(cd, trace) {
1515
for(var i = 0; i < cd.length; i++) cd[i].i = i;
1616

17-
mergeArray(trace.text, cd, 'tx');
18-
mergeArray(trace.hovertext, cd, 'htx');
17+
Lib.mergeArray(trace.text, cd, 'tx');
18+
Lib.mergeArray(trace.hovertext, cd, 'htx');
1919

2020
var marker = trace.marker;
2121
if(marker) {
22-
mergeArray(marker.opacity, cd, 'mo');
23-
mergeArray(marker.color, cd, 'mc');
22+
Lib.mergeArray(marker.opacity, cd, 'mo');
23+
Lib.mergeArray(marker.color, cd, 'mc');
2424

2525
var markerLine = marker.line;
2626
if(markerLine) {
27-
mergeArray(markerLine.color, cd, 'mlc');
28-
mergeArray(markerLine.width, cd, 'mlw');
27+
Lib.mergeArray(markerLine.color, cd, 'mlc');
28+
Lib.mergeArrayCastPositive(markerLine.width, cd, 'mlw');
2929
}
3030
}
3131
};

‎src/traces/scatter/arrays_to_calcdata.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,22 @@ module.exports = function arraysToCalcdata(cd, trace) {
2222
Lib.mergeArray(trace.customdata, cd, 'data');
2323
Lib.mergeArray(trace.textposition, cd, 'tp');
2424
if(trace.textfont) {
25-
Lib.mergeArray(trace.textfont.size, cd, 'ts');
25+
Lib.mergeArrayCastPositive(trace.textfont.size, cd, 'ts');
2626
Lib.mergeArray(trace.textfont.color, cd, 'tc');
2727
Lib.mergeArray(trace.textfont.family, cd, 'tf');
2828
}
2929

3030
var marker = trace.marker;
3131
if(marker) {
32-
Lib.mergeArray(marker.size, cd, 'ms');
33-
Lib.mergeArray(marker.opacity, cd, 'mo');
32+
Lib.mergeArrayCastPositive(marker.size, cd, 'ms');
33+
Lib.mergeArrayCastPositive(marker.opacity, cd, 'mo');
3434
Lib.mergeArray(marker.symbol, cd, 'mx');
3535
Lib.mergeArray(marker.color, cd, 'mc');
3636

3737
var markerLine = marker.line;
3838
if(marker.line) {
3939
Lib.mergeArray(markerLine.color, cd, 'mlc');
40-
Lib.mergeArray(markerLine.width, cd, 'mlw');
40+
Lib.mergeArrayCastPositive(markerLine.width, cd, 'mlw');
4141
}
4242

4343
var markerGradient = marker.gradient;

‎test/jasmine/tests/bar_test.js

+14
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,20 @@ describe('Bar.calc', function() {
407407
assertPointField(cd, 'x', [[1, NaN, NaN, 15]]);
408408
assertPointField(cd, 'y', [[1, 2, 10, 30]]);
409409
});
410+
411+
it('should guard against negative marker.line.width values', function() {
412+
var gd = mockBarPlot([{
413+
marker: {
414+
line: {
415+
width: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
416+
}
417+
},
418+
y: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
419+
}], {});
420+
421+
var cd = gd.calcdata;
422+
assertPointField(cd, 'mlw', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
423+
});
410424
});
411425

412426
describe('Bar.crossTraceCalc (formerly known as setPositions)', function() {

‎test/jasmine/tests/funnel_test.js

+14
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,20 @@ describe('Funnel.calc', function() {
351351
assertPointField(cd, 'y', [[1, NaN, NaN, 15]]);
352352
assertPointField(cd, 'x', [[0.5, 1, 5, 15]]);
353353
});
354+
355+
it('should guard against negative marker.line.width values', function() {
356+
var gd = mockFunnelPlot([{
357+
marker: {
358+
line: {
359+
width: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
360+
}
361+
},
362+
y: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
363+
}], {});
364+
365+
var cd = gd.calcdata;
366+
assertPointField(cd, 'mlw', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
367+
});
354368
});
355369

356370
describe('Funnel.crossTraceCalc', function() {

‎test/jasmine/tests/scatter_test.js

+52
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ var Scatter = require('@src/traces/scatter');
33
var makeBubbleSizeFn = require('@src/traces/scatter/make_bubble_size_func');
44
var linePoints = require('@src/traces/scatter/line_points');
55
var Lib = require('@src/lib');
6+
var Plots = require('@src/plots/plots');
67

78
var Plotly = require('@lib/index');
89
var createGraphDiv = require('../assets/create_graph_div');
@@ -18,6 +19,8 @@ var assertMultiNodeOrder = customAssertions.assertMultiNodeOrder;
1819
var checkEventData = require('../assets/check_event_data');
1920
var constants = require('@src/traces/scatter/constants');
2021

22+
var supplyAllDefaults = require('../assets/supply_defaults');
23+
2124
var getOpacity = function(node) { return Number(node.style.opacity); };
2225
var getFillOpacity = function(node) { return Number(node.style['fill-opacity']); };
2326
var getColor = function(node) { return node.style.fill; };
@@ -273,6 +276,55 @@ describe('Test scatter', function() {
273276
});
274277
});
275278

279+
describe('calc', function() {
280+
function assertPointField(calcData, prop, expectation) {
281+
var values = [];
282+
283+
calcData.forEach(function(calcTrace) {
284+
var vals = calcTrace.map(function(pt) {
285+
return Lib.nestedProperty(pt, prop).get();
286+
});
287+
288+
values.push(vals);
289+
});
290+
291+
expect(values).toBeCloseTo2DArray(expectation, undefined, '(field ' + prop + ')');
292+
}
293+
294+
it('should guard against negative size values', function() {
295+
var gd = {
296+
data: [{
297+
type: 'scatter',
298+
mode: 'markers+text',
299+
marker: {
300+
line: {
301+
width: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
302+
},
303+
opacity: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1'],
304+
size: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
305+
},
306+
textfont: {
307+
size: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
308+
},
309+
text: ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14'],
310+
y: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
311+
}],
312+
layout: {},
313+
calcdata: [],
314+
_context: {locale: 'en', locales: {}}
315+
};
316+
317+
supplyAllDefaults(gd);
318+
Plots.doCalcdata(gd);
319+
320+
var cd = gd.calcdata;
321+
assertPointField(cd, 'mlw', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
322+
assertPointField(cd, 'mo', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
323+
assertPointField(cd, 'ms', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
324+
assertPointField(cd, 'ts', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
325+
});
326+
});
327+
276328
describe('isBubble', function() {
277329
it('should return true when marker.size is an Array', function() {
278330
var trace = {

‎test/jasmine/tests/scattergeo_test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ describe('Test scattergeo calc', function() {
215215

216216
expect(calcTrace).toEqual([
217217
{ lonlat: [10, 20], mc: 0, ms: 10 },
218-
{ lonlat: [20, 30], mc: null, ms: NaN },
218+
{ lonlat: [20, 30], mc: null, ms: 0 },
219219
{ lonlat: [BADNUM, BADNUM], mc: 5, ms: 8 },
220220
{ lonlat: [30, 10], mc: 10, ms: 10 }
221221
]);

0 commit comments

Comments
 (0)