handle invalid values and zero totals for pie and funnelarea

Author: archmoj

src/traces/funnelarea/defaults.js

@@ -12,22 +12,24 @@ var Lib = require('../../lib');
 var attributes = require('./attributes');
 var handleDomainDefaults = require('../../plots/domain').defaults;
 var handleText = require('../bar/defaults').handleText;
+var handleLabelsAndValues = require('../pie/defaults').handleLabelsAndValues;
 
 module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
     function coerce(attr, dflt) {
         return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
     }
 
-    var len;
-    var vals = coerce('values');
-    var hasVals = Lib.isArrayOrTypedArray(vals);
     var labels = coerce('labels');
-    if(Array.isArray(labels)) {
-        len = labels.length;
-        if(hasVals) len = Math.min(len, vals.length);
-    } else if(hasVals) {
-        len = vals.length;
+    var values = coerce('values');
 
+    var res = handleLabelsAndValues(labels, values);
+    var len = res.len;
+    traceOut._hasLabels = res.hasLabels;
+    traceOut._hasValues = res.hasValues;
+
+    if(!traceOut._hasLabels &&
+        traceOut._hasValues
+    ) {
         coerce('label0');
         coerce('dlabel');
     }

src/traces/pie/calc.js

@@ -9,7 +9,6 @@
 'use strict';
 
 var isNumeric = require('fast-isnumeric');
-var isArrayOrTypedArray = require('../../lib').isArrayOrTypedArray;
 var tinycolor = require('tinycolor2');
 
 var Color = require('../../components/color');
@@ -25,26 +24,26 @@ function calc(gd, trace) {
     var labels = trace.labels;
     var colors = trace.marker.colors || [];
     var vals = trace.values;
-    var hasVals = isArrayOrTypedArray(vals) && vals.length;
+    var len = trace._length;
+    var hasValues = trace._hasValues && len;
 
     var i, pt;
 
     if(trace.dlabel) {
-        labels = new Array(vals.length);
-        for(i = 0; i < vals.length; i++) {
+        labels = [];
+        for(i = 0; i < len; i++) {
             labels[i] = String(trace.label0 + i * trace.dlabel);
         }
     }
 
     var allThisTraceLabels = {};
     var pullColor = makePullColorFn(fullLayout['_' + trace.type + 'colormap']);
-    var seriesLen = (hasVals ? vals : labels).length;
     var vTotal = 0;
     var isAggregated = false;
 
-    for(i = 0; i < seriesLen; i++) {
+    for(i = 0; i < len; i++) {
         var v, label, hidden;
-        if(hasVals) {
+        if(hasValues) {
             v = vals[i];
             if(!isNumeric(v)) continue;
             v = +v;

src/traces/pie/defaults.js

@@ -13,21 +13,52 @@ var attributes = require('./attributes');
 var handleDomainDefaults = require('../../plots/domain').defaults;
 var handleText = require('../bar/defaults').handleText;
 
-module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
+function handleLabelsAndValues(labels, values) {
+    var hasLabels = Array.isArray(labels);
+    var hasValues = Lib.isArrayOrTypedArray(values);
+    var len = Math.min(
+        hasLabels ? labels.length : Infinity,
+        hasValues ? values.length : Infinity
+    );
+
+    if(!isFinite(len)) len = 0;
+
+    if(len && hasValues) {
+        var sum = 0;
+        for(var i = 0; i < len; i++) {
+            var v = +values[i];
+            if(v < 0) {
+                sum = 0;
+                break;
+            }
+            sum += v;
+        }
+        if(!sum) len = 0;
+    }
+
+    return {
+        hasLabels: hasLabels,
+        hasValues: hasValues,
+        len: len
+    };
+}
+
+function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
     function coerce(attr, dflt) {
         return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
     }
 
-    var len;
-    var vals = coerce('values');
-    var hasVals = Lib.isArrayOrTypedArray(vals);
     var labels = coerce('labels');
-    if(Array.isArray(labels)) {
-        len = labels.length;
-        if(hasVals) len = Math.min(len, vals.length);
-    } else if(hasVals) {
-        len = vals.length;
+    var values = coerce('values');
+
+    var res = handleLabelsAndValues(labels, values);
+    var len = res.len;
+    traceOut._hasLabels = res.hasLabels;
+    traceOut._hasValues = res.hasValues;
 
+    if(!traceOut._hasLabels &&
+        traceOut._hasValues
+    ) {
         coerce('label0');
         coerce('dlabel');
     }
@@ -86,4 +117,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     coerce('direction');
     coerce('rotation');
     coerce('pull');
+}
+
+module.exports = {
+    handleLabelsAndValues: handleLabelsAndValues,
+    supplyDefaults: supplyDefaults
 };

src/traces/pie/index.js

@@ -10,7 +10,7 @@
 
 module.exports = {
     attributes: require('./attributes'),
-    supplyDefaults: require('./defaults'),
+    supplyDefaults: require('./defaults').supplyDefaults,
     supplyLayoutDefaults: require('./layout_defaults'),
     layoutAttributes: require('./layout_attributes'),
 

test/jasmine/tests/funnelarea_test.js

@@ -71,6 +71,44 @@ describe('Funnelarea defaults', function() {
         expect(out.visible).toBe(false);
     });
 
+    it('allows falsy json values to skip but does not allow bad values and zero sum', function() {
+        var out;
+
+        [null, 0, '', false].forEach(function(e) {
+            out = _supply({type: 'pie', values: [1, e, 3]});
+            expect(out.visible).toBe(true, e);
+            expect(out._length).toBe(3, e);
+
+            out = _supply({type: 'pie', values: [1, e]});
+            expect(out.visible).toBe(true, e);
+            expect(out._length).toBe(2, e);
+
+            out = _supply({type: 'pie', values: [0, e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+
+            out = _supply({type: 'pie', values: [e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+        });
+
+        ['not a positive value', '-1', -1, NaN, undefined].forEach(function(e) {
+            out = _supply({type: 'pie', values: [0, e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+
+            out = _supply({type: 'pie', values: [1, e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+        });
+
+        ['1', '+1', '1e1'].forEach(function(e) {
+            out = _supply({type: 'pie', values: [0, e]});
+            expect(out.visible).toBe(true, e);
+            expect(out._length).toBe(2, e);
+        });
+    });
+
     it('is marked invisible if either labels or values is empty', function() {
         var out = _supply({type: 'funnelarea', labels: [], values: [1, 2]});
         expect(out.visible).toBe(false);

test/jasmine/tests/pie_test.js

@@ -55,6 +55,44 @@ describe('Pie defaults', function() {
         expect(out.visible).toBe(false);
     });
 
+    it('allows falsy json values to skip but does not allow bad values and zero sum', function() {
+        var out;
+
+        [null, 0, '', false].forEach(function(e) {
+            out = _supply({type: 'pie', values: [1, e, 3]});
+            expect(out.visible).toBe(true, e);
+            expect(out._length).toBe(3, e);
+
+            out = _supply({type: 'pie', values: [1, e]});
+            expect(out.visible).toBe(true, e);
+            expect(out._length).toBe(2, e);
+
+            out = _supply({type: 'pie', values: [0, e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+
+            out = _supply({type: 'pie', values: [e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+        });
+
+        ['not a positive value', '-1', -1, NaN, -Infinity, undefined].forEach(function(e) {
+            out = _supply({type: 'pie', values: [0, e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+
+            out = _supply({type: 'pie', values: [1, e]});
+            expect(out.visible).toBe(false, e);
+            expect(out._length).toBe(undefined, e);
+        });
+
+        ['1', '+1', '1e1'].forEach(function(e) {
+            out = _supply({type: 'pie', values: [0, e]});
+            expect(out.visible).toBe(true, e);
+            expect(out._length).toBe(2, e);
+        });
+    });
+
     it('is marked invisible if either labels or values is empty', function() {
         var out = _supply({type: 'pie', labels: [], values: [1, 2]});
         expect(out.visible).toBe(false);