Skip to content

Commit dfc1439

Browse files
archmojarchmoj
committed
handle invalid values and zero totals for pie and funnelarea
1 parent 0be6afd commit dfc1439

File tree

6 files changed

+138
-25
lines changed

6 files changed

+138
-25
lines changed

‎src/traces/funnelarea/defaults.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,24 @@ var Lib = require('../../lib');
1212
var attributes = require('./attributes');
1313
var handleDomainDefaults = require('../../plots/domain').defaults;
1414
var handleText = require('../bar/defaults').handleText;
15+
var handleLabelsAndValues = require('../pie/defaults').handleLabelsAndValues;
1516

1617
module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
1718
function coerce(attr, dflt) {
1819
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
1920
}
2021

21-
var len;
22-
var vals = coerce('values');
23-
var hasVals = Lib.isArrayOrTypedArray(vals);
2422
var labels = coerce('labels');
25-
if(Array.isArray(labels)) {
26-
len = labels.length;
27-
if(hasVals) len = Math.min(len, vals.length);
28-
} else if(hasVals) {
29-
len = vals.length;
23+
var values = coerce('values');
3024

25+
var res = handleLabelsAndValues(labels, values);
26+
var len = res.len;
27+
traceOut._hasLabels = res.hasLabels;
28+
traceOut._hasValues = res.hasValues;
29+
30+
if(!traceOut._hasLabels &&
31+
traceOut._hasValues
32+
) {
3133
coerce('label0');
3234
coerce('dlabel');
3335
}

‎src/traces/pie/calc.js‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
'use strict';
1010

1111
var isNumeric = require('fast-isnumeric');
12-
var isArrayOrTypedArray = require('../../lib').isArrayOrTypedArray;
1312
var tinycolor = require('tinycolor2');
1413

1514
var Color = require('../../components/color');
@@ -25,26 +24,26 @@ function calc(gd, trace) {
2524
var labels = trace.labels;
2625
var colors = trace.marker.colors || [];
2726
var vals = trace.values;
28-
var hasVals = isArrayOrTypedArray(vals) && vals.length;
27+
var len = trace._length;
28+
var hasValues = trace._hasValues && len;
2929

3030
var i, pt;
3131

3232
if(trace.dlabel) {
33-
labels = new Array(vals.length);
34-
for(i = 0; i < vals.length; i++) {
33+
labels = [];
34+
for(i = 0; i < len; i++) {
3535
labels[i] = String(trace.label0 + i * trace.dlabel);
3636
}
3737
}
3838

3939
var allThisTraceLabels = {};
4040
var pullColor = makePullColorFn(fullLayout['_' + trace.type + 'colormap']);
41-
var seriesLen = (hasVals ? vals : labels).length;
4241
var vTotal = 0;
4342
var isAggregated = false;
4443

45-
for(i = 0; i < seriesLen; i++) {
44+
for(i = 0; i < len; i++) {
4645
var v, label, hidden;
47-
if(hasVals) {
46+
if(hasValues) {
4847
v = vals[i];
4948
if(!isNumeric(v)) continue;
5049
v = +v;

‎src/traces/pie/defaults.js‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,52 @@ var attributes = require('./attributes');
1313
var handleDomainDefaults = require('../../plots/domain').defaults;
1414
var handleText = require('../bar/defaults').handleText;
1515

16-
module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
16+
function handleLabelsAndValues(labels, values) {
17+
var hasLabels = Array.isArray(labels);
18+
var hasValues = Lib.isArrayOrTypedArray(values);
19+
var len = Math.min(
20+
hasLabels ? labels.length : Infinity,
21+
hasValues ? values.length : Infinity
22+
);
23+
24+
if(!isFinite(len)) len = 0;
25+
26+
if(len && hasValues) {
27+
var sum = 0;
28+
for(var i = 0; i < len; i++) {
29+
var v = +values[i];
30+
if(v < 0) {
31+
sum = 0;
32+
break;
33+
}
34+
sum += v;
35+
}
36+
if(!sum) len = 0;
37+
}
38+
39+
return {
40+
hasLabels: hasLabels,
41+
hasValues: hasValues,
42+
len: len
43+
};
44+
}
45+
46+
function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
1747
function coerce(attr, dflt) {
1848
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
1949
}
2050

21-
var len;
22-
var vals = coerce('values');
23-
var hasVals = Lib.isArrayOrTypedArray(vals);
2451
var labels = coerce('labels');
25-
if(Array.isArray(labels)) {
26-
len = labels.length;
27-
if(hasVals) len = Math.min(len, vals.length);
28-
} else if(hasVals) {
29-
len = vals.length;
52+
var values = coerce('values');
53+
54+
var res = handleLabelsAndValues(labels, values);
55+
var len = res.len;
56+
traceOut._hasLabels = res.hasLabels;
57+
traceOut._hasValues = res.hasValues;
3058

59+
if(!traceOut._hasLabels &&
60+
traceOut._hasValues
61+
) {
3162
coerce('label0');
3263
coerce('dlabel');
3364
}
@@ -86,4 +117,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
86117
coerce('direction');
87118
coerce('rotation');
88119
coerce('pull');
120+
}
121+
122+
module.exports = {
123+
handleLabelsAndValues: handleLabelsAndValues,
124+
supplyDefaults: supplyDefaults
89125
};

‎src/traces/pie/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
module.exports = {
1212
attributes: require('./attributes'),
13-
supplyDefaults: require('./defaults'),
13+
supplyDefaults: require('./defaults').supplyDefaults,
1414
supplyLayoutDefaults: require('./layout_defaults'),
1515
layoutAttributes: require('./layout_attributes'),
1616

‎test/jasmine/tests/funnelarea_test.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,44 @@ describe('Funnelarea defaults', function() {
7171
expect(out.visible).toBe(false);
7272
});
7373

74+
it('allows falsy json values to skip but does not allow bad values and zero sum', function() {
75+
var out;
76+
77+
[null, 0, '', false].forEach(function(e) {
78+
out = _supply({type: 'pie', values: [1, e, 3]});
79+
expect(out.visible).toBe(true, e);
80+
expect(out._length).toBe(3, e);
81+
82+
out = _supply({type: 'pie', values: [1, e]});
83+
expect(out.visible).toBe(true, e);
84+
expect(out._length).toBe(2, e);
85+
86+
out = _supply({type: 'pie', values: [0, e]});
87+
expect(out.visible).toBe(false, e);
88+
expect(out._length).toBe(undefined, e);
89+
90+
out = _supply({type: 'pie', values: [e]});
91+
expect(out.visible).toBe(false, e);
92+
expect(out._length).toBe(undefined, e);
93+
});
94+
95+
['not a positive value', '-1', -1, NaN, undefined].forEach(function(e) {
96+
out = _supply({type: 'pie', values: [0, e]});
97+
expect(out.visible).toBe(false, e);
98+
expect(out._length).toBe(undefined, e);
99+
100+
out = _supply({type: 'pie', values: [1, e]});
101+
expect(out.visible).toBe(false, e);
102+
expect(out._length).toBe(undefined, e);
103+
});
104+
105+
['1', '+1', '1e1'].forEach(function(e) {
106+
out = _supply({type: 'pie', values: [0, e]});
107+
expect(out.visible).toBe(true, e);
108+
expect(out._length).toBe(2, e);
109+
});
110+
});
111+
74112
it('is marked invisible if either labels or values is empty', function() {
75113
var out = _supply({type: 'funnelarea', labels: [], values: [1, 2]});
76114
expect(out.visible).toBe(false);

‎test/jasmine/tests/pie_test.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,44 @@ describe('Pie defaults', function() {
5555
expect(out.visible).toBe(false);
5656
});
5757

58+
it('allows falsy json values to skip but does not allow bad values and zero sum', function() {
59+
var out;
60+
61+
[null, 0, '', false].forEach(function(e) {
62+
out = _supply({type: 'pie', values: [1, e, 3]});
63+
expect(out.visible).toBe(true, e);
64+
expect(out._length).toBe(3, e);
65+
66+
out = _supply({type: 'pie', values: [1, e]});
67+
expect(out.visible).toBe(true, e);
68+
expect(out._length).toBe(2, e);
69+
70+
out = _supply({type: 'pie', values: [0, e]});
71+
expect(out.visible).toBe(false, e);
72+
expect(out._length).toBe(undefined, e);
73+
74+
out = _supply({type: 'pie', values: [e]});
75+
expect(out.visible).toBe(false, e);
76+
expect(out._length).toBe(undefined, e);
77+
});
78+
79+
['not a positive value', '-1', -1, NaN, -Infinity, undefined].forEach(function(e) {
80+
out = _supply({type: 'pie', values: [0, e]});
81+
expect(out.visible).toBe(false, e);
82+
expect(out._length).toBe(undefined, e);
83+
84+
out = _supply({type: 'pie', values: [1, e]});
85+
expect(out.visible).toBe(false, e);
86+
expect(out._length).toBe(undefined, e);
87+
});
88+
89+
['1', '+1', '1e1'].forEach(function(e) {
90+
out = _supply({type: 'pie', values: [0, e]});
91+
expect(out.visible).toBe(true, e);
92+
expect(out._length).toBe(2, e);
93+
});
94+
});
95+
5896
it('is marked invisible if either labels or values is empty', function() {
5997
var out = _supply({type: 'pie', labels: [], values: [1, 2]});
6098
expect(out.visible).toBe(false);

0 commit comments

Comments
 (0)