Merge pull request plotly#4063 from plotly/reuse-merge-array-cast-positive

Filter numbers during calc step

Author: archmoj

src/lib/index.js

@@ -474,7 +474,7 @@ lib.mergeArray = function(traceAttr, cd, cdAttr, fn) {
 lib.mergeArrayCastPositive = function(traceAttr, cd, cdAttr) {
     return lib.mergeArray(traceAttr, cd, cdAttr, function(v) {
         var w = +v;
-        return w > 0 ? w : 0;
+        return !isFinite(w) ? 0 : w > 0 ? w : 0;
     });
 };
 

src/traces/funnel/arrays_to_calcdata.js

@@ -8,24 +8,24 @@
 
 'use strict';
 
-var mergeArray = require('../../lib').mergeArray;
+var Lib = require('../../lib');
 
 // arrayOk attributes, merge them into calcdata array
 module.exports = function arraysToCalcdata(cd, trace) {
     for(var i = 0; i < cd.length; i++) cd[i].i = i;
 
-    mergeArray(trace.text, cd, 'tx');
-    mergeArray(trace.hovertext, cd, 'htx');
+    Lib.mergeArray(trace.text, cd, 'tx');
+    Lib.mergeArray(trace.hovertext, cd, 'htx');
 
     var marker = trace.marker;
     if(marker) {
-        mergeArray(marker.opacity, cd, 'mo');
-        mergeArray(marker.color, cd, 'mc');
+        Lib.mergeArray(marker.opacity, cd, 'mo');
+        Lib.mergeArray(marker.color, cd, 'mc');
 
         var markerLine = marker.line;
         if(markerLine) {
-            mergeArray(markerLine.color, cd, 'mlc');
-            mergeArray(markerLine.width, cd, 'mlw');
+            Lib.mergeArray(markerLine.color, cd, 'mlc');
+            Lib.mergeArrayCastPositive(markerLine.width, cd, 'mlw');
         }
     }
 };

src/traces/scatter/arrays_to_calcdata.js

@@ -22,22 +22,22 @@ module.exports = function arraysToCalcdata(cd, trace) {
     Lib.mergeArray(trace.customdata, cd, 'data');
     Lib.mergeArray(trace.textposition, cd, 'tp');
     if(trace.textfont) {
-        Lib.mergeArray(trace.textfont.size, cd, 'ts');
+        Lib.mergeArrayCastPositive(trace.textfont.size, cd, 'ts');
         Lib.mergeArray(trace.textfont.color, cd, 'tc');
         Lib.mergeArray(trace.textfont.family, cd, 'tf');
     }
 
     var marker = trace.marker;
     if(marker) {
-        Lib.mergeArray(marker.size, cd, 'ms');
-        Lib.mergeArray(marker.opacity, cd, 'mo');
+        Lib.mergeArrayCastPositive(marker.size, cd, 'ms');
+        Lib.mergeArrayCastPositive(marker.opacity, cd, 'mo');
         Lib.mergeArray(marker.symbol, cd, 'mx');
         Lib.mergeArray(marker.color, cd, 'mc');
 
         var markerLine = marker.line;
         if(marker.line) {
             Lib.mergeArray(markerLine.color, cd, 'mlc');
-            Lib.mergeArray(markerLine.width, cd, 'mlw');
+            Lib.mergeArrayCastPositive(markerLine.width, cd, 'mlw');
         }
 
         var markerGradient = marker.gradient;

test/jasmine/tests/bar_test.js

@@ -407,6 +407,20 @@ describe('Bar.calc', function() {
         assertPointField(cd, 'x', [[1, NaN, NaN, 15]]);
         assertPointField(cd, 'y', [[1, 2, 10, 30]]);
     });
+
+    it('should guard against negative marker.line.width values', function() {
+        var gd = mockBarPlot([{
+            marker: {
+                line: {
+                    width: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
+                }
+            },
+            y: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+        }], {});
+
+        var cd = gd.calcdata;
+        assertPointField(cd, 'mlw', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
+    });
 });
 
 describe('Bar.crossTraceCalc (formerly known as setPositions)', function() {

test/jasmine/tests/funnel_test.js

@@ -351,6 +351,20 @@ describe('Funnel.calc', function() {
         assertPointField(cd, 'y', [[1, NaN, NaN, 15]]);
         assertPointField(cd, 'x', [[0.5, 1, 5, 15]]);
     });
+
+    it('should guard against negative marker.line.width values', function() {
+        var gd = mockFunnelPlot([{
+            marker: {
+                line: {
+                    width: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
+                }
+            },
+            y: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+        }], {});
+
+        var cd = gd.calcdata;
+        assertPointField(cd, 'mlw', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
+    });
 });
 
 describe('Funnel.crossTraceCalc', function() {

test/jasmine/tests/scatter_test.js

@@ -3,6 +3,7 @@ var Scatter = require('@src/traces/scatter');
 var makeBubbleSizeFn = require('@src/traces/scatter/make_bubble_size_func');
 var linePoints = require('@src/traces/scatter/line_points');
 var Lib = require('@src/lib');
+var Plots = require('@src/plots/plots');
 
 var Plotly = require('@lib/index');
 var createGraphDiv = require('../assets/create_graph_div');
@@ -18,6 +19,8 @@ var assertMultiNodeOrder = customAssertions.assertMultiNodeOrder;
 var checkEventData = require('../assets/check_event_data');
 var constants = require('@src/traces/scatter/constants');
 
+var supplyAllDefaults = require('../assets/supply_defaults');
+
 var getOpacity = function(node) { return Number(node.style.opacity); };
 var getFillOpacity = function(node) { return Number(node.style['fill-opacity']); };
 var getColor = function(node) { return node.style.fill; };
@@ -273,6 +276,55 @@ describe('Test scatter', function() {
         });
     });
 
+    describe('calc', function() {
+        function assertPointField(calcData, prop, expectation) {
+            var values = [];
+
+            calcData.forEach(function(calcTrace) {
+                var vals = calcTrace.map(function(pt) {
+                    return Lib.nestedProperty(pt, prop).get();
+                });
+
+                values.push(vals);
+            });
+
+            expect(values).toBeCloseTo2DArray(expectation, undefined, '(field ' + prop + ')');
+        }
+
+        it('should guard against negative size values', function() {
+            var gd = {
+                data: [{
+                    type: 'scatter',
+                    mode: 'markers+text',
+                    marker: {
+                        line: {
+                            width: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
+                        },
+                        opacity: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1'],
+                        size: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
+                    },
+                    textfont: {
+                        size: [2, 1, 0, -1, false, true, null, [], -Infinity, Infinity, NaN, {}, '12+1', '1e1']
+                    },
+                    text: ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14'],
+                    y: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+                }],
+                layout: {},
+                calcdata: [],
+                _context: {locale: 'en', locales: {}}
+            };
+
+            supplyAllDefaults(gd);
+            Plots.doCalcdata(gd);
+
+            var cd = gd.calcdata;
+            assertPointField(cd, 'mlw', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
+            assertPointField(cd, 'mo', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
+            assertPointField(cd, 'ms', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
+            assertPointField(cd, 'ts', [[2, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 10]]);
+        });
+    });
+
     describe('isBubble', function() {
         it('should return true when marker.size is an Array', function() {
             var trace = {

test/jasmine/tests/scattergeo_test.js

@@ -215,7 +215,7 @@ describe('Test scattergeo calc', function() {
 
         expect(calcTrace).toEqual([
             { lonlat: [10, 20], mc: 0, ms: 10 },
-            { lonlat: [20, 30], mc: null, ms: NaN },
+            { lonlat: [20, 30], mc: null, ms: 0 },
             { lonlat: [BADNUM, BADNUM], mc: 5, ms: 8 },
             { lonlat: [30, 10], mc: 10, ms: 10 }
         ]);