Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 96fe262

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

File tree

Expand file treeCollapse file tree

7 files changed

+93
-13
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+93
-13
lines changed

‎src/lib/index.js

Copy file name to clipboardExpand all lines: src/lib/index.js
+1-1Lines changed: 1 addition & 1 deletion
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

Copy file name to clipboardExpand all lines: src/traces/funnel/arrays_to_calcdata.js
+7-7Lines changed: 7 additions & 7 deletions
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

Copy file name to clipboardExpand all lines: src/traces/scatter/arrays_to_calcdata.js
+4-4Lines changed: 4 additions & 4 deletions
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

Copy file name to clipboardExpand all lines: test/jasmine/tests/bar_test.js
+14Lines changed: 14 additions & 0 deletions
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

Copy file name to clipboardExpand all lines: test/jasmine/tests/funnel_test.js
+14Lines changed: 14 additions & 0 deletions
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

Copy file name to clipboardExpand all lines: test/jasmine/tests/scatter_test.js
+52Lines changed: 52 additions & 0 deletions
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

Copy file name to clipboardExpand all lines: test/jasmine/tests/scattergeo_test.js
+1-1Lines changed: 1 addition & 1 deletion
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.