Skip to content

Commit c70f7d5

Browse files
committed
do not coerce scale* attrs when violin 'width' is set
- do not fill in fullLayout._violinScaleGroupStats for traces with set 'width' - use maxKDE key instead of maxWidth to avoid confusing
1 parent d1aed48 commit c70f7d5

File tree

4 files changed

+49
-26
lines changed

4 files changed

+49
-26
lines changed

src/traces/violin/calc.js

+22-13
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,10 @@ module.exports = function calc(gd, trace) {
2525
trace[trace.orientation === 'h' ? 'xaxis' : 'yaxis']
2626
);
2727

28-
var violinScaleGroupStats = fullLayout._violinScaleGroupStats;
29-
var scaleGroup = trace.scalegroup;
30-
var groupStats = violinScaleGroupStats[scaleGroup];
31-
if(!groupStats) {
32-
groupStats = violinScaleGroupStats[scaleGroup] = {
33-
maxWidth: 0,
34-
maxCount: 0
35-
};
36-
}
37-
3828
var spanMin = Infinity;
3929
var spanMax = -Infinity;
30+
var maxKDE = 0;
31+
var maxCount = 0;
4032

4133
for(var i = 0; i < cd.length; i++) {
4234
var cdi = cd[i];
@@ -61,19 +53,36 @@ module.exports = function calc(gd, trace) {
6153

6254
for(var k = 0, t = span[0]; t < (span[1] + step / 2); k++, t += step) {
6355
var v = kde(t);
64-
groupStats.maxWidth = Math.max(groupStats.maxWidth, v);
6556
cdi.density[k] = {v: v, t: t};
57+
maxKDE = Math.max(maxKDE, v);
6658
}
6759

68-
groupStats.maxCount = Math.max(groupStats.maxCount, vals.length);
69-
60+
maxCount = Math.max(maxCount, vals.length);
7061
spanMin = Math.min(spanMin, span[0]);
7162
spanMax = Math.max(spanMax, span[1]);
7263
}
7364

7465
var extremes = Axes.findExtremes(valAxis, [spanMin, spanMax], {padded: true});
7566
trace._extremes[valAxis._id] = extremes;
7667

68+
if(trace.width) {
69+
cd[0].t.maxKDE = maxKDE;
70+
} else {
71+
var violinScaleGroupStats = fullLayout._violinScaleGroupStats;
72+
var scaleGroup = trace.scalegroup;
73+
var groupStats = violinScaleGroupStats[scaleGroup];
74+
75+
if(groupStats) {
76+
groupStats.maxKDE = Math.max(groupStats.maxKDE, maxKDE);
77+
groupStats.maxCount = Math.max(groupStats.maxCount, maxCount);
78+
} else {
79+
violinScaleGroupStats[scaleGroup] = {
80+
maxKDE: maxKDE,
81+
maxCount: maxCount
82+
};
83+
}
84+
}
85+
7786
cd[0].t.labels.kde = Lib._(gd, 'kde:');
7887

7988
return cd;

src/traces/violin/defaults.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
2727

2828
coerce('bandwidth');
2929
coerce('side');
30+
3031
var width = coerce('width');
3132
if(!width) {
3233
coerce('scalegroup', traceOut.name);
3334
coerce('scalemode');
34-
} else {
35-
traceOut.scalegroup = '';
36-
traceOut.scalemode = 'width';
3735
}
3836

3937
var span = coerce('span');

src/traces/violin/plot.js

+8-9
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
7878
var hasBothSides = trace.side === 'both';
7979
var hasPositiveSide = hasBothSides || trace.side === 'positive';
8080
var hasNegativeSide = hasBothSides || trace.side === 'negative';
81-
var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup];
8281

8382
var violins = plotGroup.selectAll('path.violin').data(Lib.identity);
8483

@@ -94,15 +93,15 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
9493
var len = density.length;
9594
var posCenter = d.pos + bPos;
9695
var posCenterPx = posAxis.c2p(posCenter);
97-
var scale;
9896

99-
switch(trace.scalemode) {
100-
case 'width':
101-
scale = groupStats.maxWidth / bdPos;
102-
break;
103-
case 'count':
104-
scale = (groupStats.maxWidth / bdPos) * (groupStats.maxCount / d.pts.length);
105-
break;
97+
var scale;
98+
if(trace.width) {
99+
scale = t.maxKDE / bdPos;
100+
} else {
101+
var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup];
102+
scale = trace.scalemode === 'count' ?
103+
(groupStats.maxKDE / bdPos) * (groupStats.maxCount / d.pts.length) :
104+
groupStats.maxKDE / bdPos;
106105
}
107106

108107
var pathPos, pathNeg, path;

test/jasmine/tests/violin_test.js

+18-1
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,23 @@ describe('Test violin defaults', function() {
142142
expect(traceOut.meanline.color).toBe('blue');
143143
expect(traceOut.meanline.width).toBe(10);
144144
});
145+
146+
it('should not coerce *scalegroup* and *scalemode* when *width* is set', function() {
147+
_supply({
148+
y: [1, 2, 1],
149+
width: 1
150+
});
151+
expect(traceOut.scalemode).toBeUndefined();
152+
expect(traceOut.scalegroup).toBeUndefined();
153+
154+
_supply({
155+
y: [1, 2, 1],
156+
// width=0 is ignored during calc
157+
width: 0
158+
});
159+
expect(traceOut.scalemode).toBe('width');
160+
expect(traceOut.scalegroup).toBe('');
161+
});
145162
});
146163

147164
describe('Test violin calc:', function() {
@@ -236,7 +253,7 @@ describe('Test violin calc:', function() {
236253
name: 'one',
237254
y: [0, 0, 0, 0, 10, 10, 10, 10]
238255
});
239-
expect(fullLayout._violinScaleGroupStats.one.maxWidth).toBeCloseTo(0.055);
256+
expect(fullLayout._violinScaleGroupStats.one.maxKDE).toBeCloseTo(0.055);
240257
expect(fullLayout._violinScaleGroupStats.one.maxCount).toBe(8);
241258
});
242259

0 commit comments

Comments
 (0)