Skip to content

Commit 9fc2c82

Browse files
archmojarchmoj
authored
Merge pull request #4416 from plotly/fix4414-pie-handle-invalid-values-and-zero-total
Handle invalid values and zero totals for pie and funnelarea
2 parents ba8d941 + 1967fd4 commit 9fc2c82

File tree

6 files changed

+148
-25
lines changed

6 files changed

+148
-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 = new Array(len);
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
@@ -8,26 +8,57 @@
88

99
'use strict';
1010

11+
var isNumeric = require('fast-isnumeric');
1112
var Lib = require('../../lib');
1213
var attributes = require('./attributes');
1314
var handleDomainDefaults = require('../../plots/domain').defaults;
1415
var handleText = require('../bar/defaults').handleText;
1516

16-
module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
17+
function handleLabelsAndValues(labels, values) {
18+
var hasLabels = Array.isArray(labels);
19+
var hasValues = Lib.isArrayOrTypedArray(values);
20+
var len = Math.min(
21+
hasLabels ? labels.length : Infinity,
22+
hasValues ? values.length : Infinity
23+
);
24+
25+
if(!isFinite(len)) len = 0;
26+
27+
if(len && hasValues) {
28+
var hasPositive;
29+
for(var i = 0; i < len; i++) {
30+
var v = values[i];
31+
if(isNumeric(v) && v > 0) {
32+
hasPositive = true;
33+
break;
34+
}
35+
}
36+
if(!hasPositive) 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
}
@@ -90,4 +121,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
90121
coerce('direction');
91122
coerce('rotation');
92123
coerce('pull');
124+
}
125+
126+
module.exports = {
127+
handleLabelsAndValues: handleLabelsAndValues,
128+
supplyDefaults: supplyDefaults
93129
};

‎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: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,49 @@ describe('Funnelarea defaults', function() {
7171
expect(out.visible).toBe(false);
7272
});
7373

74+
it('skip negatives and non-JSON values and avoid zero total', function() {
75+
[
76+
-1, '-1',
77+
0, '0',
78+
false, 'false',
79+
true, 'true',
80+
null, 'null',
81+
NaN, 'NaN',
82+
-Infinity, '-Infinity',
83+
Infinity, 'Infinity',
84+
undefined, 'undefined',
85+
'', [], {}
86+
].forEach(function(e) {
87+
var out;
88+
89+
out = _supply({type: 'pie', values: [1, e, 3]});
90+
expect(out.visible).toBe(true, e);
91+
expect(out._length).toBe(3, e);
92+
93+
out = _supply({type: 'pie', values: [1, e]});
94+
expect(out.visible).toBe(true, e);
95+
expect(out._length).toBe(2, e);
96+
97+
out = _supply({type: 'pie', values: [0, e]});
98+
expect(out.visible).toBe(false, e);
99+
expect(out._length).toBe(undefined, e);
100+
101+
out = _supply({type: 'pie', values: [e]});
102+
expect(out.visible).toBe(false, e);
103+
expect(out._length).toBe(undefined, e);
104+
});
105+
});
106+
107+
it('convert positive numbers in string format', function() {
108+
['1', '+1', '1e1'].forEach(function(e) {
109+
var out;
110+
111+
out = _supply({type: 'pie', values: [0, e]});
112+
expect(out.visible).toBe(true, e);
113+
expect(out._length).toBe(2, e);
114+
});
115+
});
116+
74117
it('is marked invisible if either labels or values is empty', function() {
75118
var out = _supply({type: 'funnelarea', labels: [], values: [1, 2]});
76119
expect(out.visible).toBe(false);

‎test/jasmine/tests/pie_test.js‎

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

58+
it('skip negatives and non-JSON values and avoid zero total', function() {
59+
[
60+
-1, '-1',
61+
0, '0',
62+
false, 'false',
63+
true, 'true',
64+
null, 'null',
65+
NaN, 'NaN',
66+
-Infinity, '-Infinity',
67+
Infinity, 'Infinity',
68+
undefined, 'undefined',
69+
'', [], {}
70+
].forEach(function(e) {
71+
var out;
72+
73+
out = _supply({type: 'pie', values: [1, e, 3]});
74+
expect(out.visible).toBe(true, e);
75+
expect(out._length).toBe(3, e);
76+
77+
out = _supply({type: 'pie', values: [1, e]});
78+
expect(out.visible).toBe(true, e);
79+
expect(out._length).toBe(2, e);
80+
81+
out = _supply({type: 'pie', values: [0, e]});
82+
expect(out.visible).toBe(false, e);
83+
expect(out._length).toBe(undefined, e);
84+
85+
out = _supply({type: 'pie', values: [e]});
86+
expect(out.visible).toBe(false, e);
87+
expect(out._length).toBe(undefined, e);
88+
});
89+
});
90+
91+
it('convert positive numbers in string format', function() {
92+
['1', '+1', '1e1'].forEach(function(e) {
93+
var out;
94+
95+
out = _supply({type: 'pie', values: [0, e]});
96+
expect(out.visible).toBe(true, e);
97+
expect(out._length).toBe(2, e);
98+
});
99+
});
100+
58101
it('is marked invisible if either labels or values is empty', function() {
59102
var out = _supply({type: 'pie', labels: [], values: [1, 2]});
60103
expect(out.visible).toBe(false);

0 commit comments

Comments
 (0)