Skip to content

add 'width' to box and violin trace #3109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 40 commits into from
Closed
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
8f04d13
added vwidth param - not working
Coding-with-Adam Oct 12, 2018
f76ca97
update dPos with vwidth//remove most console.logs
Coding-with-Adam Oct 15, 2018
81cdbe9
some revision based on review comments
Coding-with-Adam Oct 17, 2018
5ef2eeb
change dflt vwidth to 0;assign dPos for each trace
Coding-with-Adam Oct 18, 2018
1c6b26b
vwidth is now the half width // padding now uses trace.width values i…
Coding-with-Adam Oct 19, 2018
2d93a0e
vwidth to width
Coding-with-Adam Oct 19, 2018
1f9ced8
compact the t.dPos assignment with trace.width
Coding-with-Adam Oct 19, 2018
26f44c4
change location of width attribute description to be consistent with …
Coding-with-Adam Oct 19, 2018
77b9c9d
more complete descriptions
Coding-with-Adam Oct 19, 2018
243a369
add 1 image test
Coding-with-Adam Oct 22, 2018
eb5e9da
add new line to joyplot mock file
Coding-with-Adam Oct 22, 2018
eea273b
camelCase
Coding-with-Adam Oct 24, 2018
408aacb
zapped trailing ',' in box/violin attributes
Coding-with-Adam Oct 24, 2018
0555283
do not coerce scale{mode, group} if no width; otherwise set to dflt v…
Coding-with-Adam Oct 24, 2018
4eb8b44
first pass at calculating extreames for each trace
Coding-with-Adam Oct 24, 2018
44e0a88
fix up the syntax of violin/defaults
Coding-with-Adam Oct 24, 2018
f8b963b
remove old baseline png
Coding-with-Adam Oct 24, 2018
54aa355
generated updated joyplot baseline image
Coding-with-Adam Oct 24, 2018
0d4239b
remove padding depending on trace.side//regen baseline
Coding-with-Adam Oct 24, 2018
fa3a314
fix space syntax error
Coding-with-Adam Oct 24, 2018
c63780f
...
Coding-with-Adam Oct 31, 2018
c24330d
update algo for padding
Coding-with-Adam Nov 1, 2018
6e7883d
fix test syntax
Coding-with-Adam Nov 1, 2018
37786d6
remove consolve logs
Coding-with-Adam Nov 1, 2018
45ae47d
update kde line test
Coding-with-Adam Nov 1, 2018
8366c88
more testing with jasmine
Coding-with-Adam Nov 1, 2018
ce81377
remove commented lines
Coding-with-Adam Nov 1, 2018
8885b5e
remove another commented line and a useless loop
Coding-with-Adam Nov 1, 2018
57cc05a
remove maxHalfWidth var
Coding-with-Adam Nov 1, 2018
fe3d7ed
cleaner code for vpad assignment
Coding-with-Adam Nov 1, 2018
44fa269
inherit violin 'width' from box.width
etpinard Nov 6, 2018
f5ba38c
rm useless attrs in test mock
etpinard Nov 6, 2018
34722d3
mention that 'width' is in data coordinates
etpinard Nov 6, 2018
d1aed48
ignore box/violin *gap and *groupgap when 'width' is set
etpinard Nov 6, 2018
c70f7d5
do not coerce scale* attrs when violin 'width' is set
etpinard Nov 6, 2018
0dea0e9
Merge pull request #3222 from plotly/joyplots-etpinard
Kully Nov 7, 2018
5e5f9ee
vpadplus and minus dont cut off jitter:0 points on violin
Coding-with-Adam Nov 7, 2018
d04c60c
Merge branch 'joyplots' of https://github.com/plotly/plotly.js into j…
Coding-with-Adam Nov 7, 2018
91497e5
fix incorrect var name
Coding-with-Adam Nov 7, 2018
657848f
small syntax - infix operator spacing
Coding-with-Adam Nov 7, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/traces/box/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,20 @@ module.exports = {
'the vertical (horizontal).'
].join(' ')
},

width: {
valType: 'number',
min: 0,
role: 'info',
dflt: 0,
editType: 'calc',
description: [
'Sets the width of the box in data coordinate',
'If *0* (default value) the width is automatically selected based on the positions',
'of other box traces in the same subplot.'
].join(' ')
},

marker: {
outliercolor: {
valType: 'color',
Expand Down Expand Up @@ -244,7 +258,6 @@ module.exports = {
marker: scatterAttrs.unselected.marker,
editType: 'style'
},

hoveron: {
valType: 'flaglist',
flags: ['boxes', 'points'],
Expand Down
32 changes: 21 additions & 11 deletions src/traces/box/cross_trace_calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ var Lib = require('../../lib');

var orientations = ['v', 'h'];


function getPosition(di) {
return di.pos;
}

function crossTraceCalc(gd, plotinfo) {
var calcdata = gd.calcdata;
var xa = plotinfo.xaxis;
Expand Down Expand Up @@ -90,19 +95,24 @@ function setPositionOffset(traceType, gd, boxList, posAxis, pad) {
var groupgap = fullLayout[traceType + 'groupgap'];
var padfactor = (1 - gap) * (1 - groupgap) * dPos / fullLayout[numKey];

// autoscale the x axis - including space for points if they're off the side
// TODO: this will overdo it if the outermost boxes don't have
// their points as far out as the other boxes
var extremes = Axes.findExtremes(posAxis, boxdv.vals, {
vpadminus: dPos + pad[0] * padfactor,
vpadplus: dPos + pad[1] * padfactor
});

// Find maximum trace width
// we baseline this at dPos
for(i = 0; i < boxList.length; i++) {
calcTrace = calcdata[boxList[i]];
// set the width of all boxes
calcTrace[0].t.dPos = dPos;
// link extremes to all boxes
// set the width of this box
// override dPos with trace.width if present
var thisDPos = calcTrace[0].t.dPos = (calcTrace[0].trace.width / 2) || dPos;
var positions = calcTrace.map(getPosition);
// autoscale the x axis - including space for points if they're off the side
// TODO: this will overdo it if the outermost boxes don't have
// their points as far out as the other boxes
var side = calcTrace[0].trace.side;
var vpadminus = (side === 'positive') ? 0 : (thisDPos + pad[0] * padfactor);
var vpadplus = (side === 'negative') ? 0 : (thisDPos + pad[1] * padfactor);
var extremes = Axes.findExtremes(posAxis, positions, {
vpadminus: vpadminus,
vpadplus: vpadplus
});
calcTrace[0].trace._extremes[posAxis._id] = extremes;
}

Expand Down
1 change: 1 addition & 0 deletions src/traces/box/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) {

coerce('whiskerwidth');
coerce('boxmean');
coerce('width');

var notched = coerce('notched', traceIn.notchwidth !== undefined);
if(notched) coerce('notchwidth');
Expand Down
9 changes: 6 additions & 3 deletions src/traces/box/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ module.exports = {
'If *group*, the boxes are plotted next to one another',
'centered around the shared location.',
'If *overlay*, the boxes are plotted over one another,',
'you might need to set *opacity* to see them multiple boxes.'
'you might need to set *opacity* to see them multiple boxes.',
'Has no effect on traces that have *width* set.'
].join(' ')
},
boxgap: {
Expand All @@ -34,7 +35,8 @@ module.exports = {
editType: 'calc',
description: [
'Sets the gap (in plot fraction) between boxes of',
'adjacent location coordinates.'
'adjacent location coordinates.',
'Has no effect on traces that have *width* set.'
].join(' ')
},
boxgroupgap: {
Expand All @@ -46,7 +48,8 @@ module.exports = {
editType: 'calc',
description: [
'Sets the gap (in plot fraction) between boxes of',
'the same location coordinate.'
'the same location coordinate.',
'Has no effect on traces that have *width* set.'
].join(' ')
}
};
21 changes: 17 additions & 4 deletions src/traces/box/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,32 @@ function plot(gd, plotinfo, cdbox, boxLayer) {
var xa = plotinfo.xaxis;
var ya = plotinfo.yaxis;
var numBoxes = fullLayout._numBoxes;
var groupFraction = (1 - fullLayout.boxgap);
var group = (fullLayout.boxmode === 'group' && numBoxes > 1);
var groupFraction = (1 - fullLayout.boxgap);
var groupGapFraction = 1 - fullLayout.boxgroupgap;

Lib.makeTraceGroups(boxLayer, cdbox, 'trace boxes').each(function(cd) {
var plotGroup = d3.select(this);
var cd0 = cd[0];
var t = cd0.t;
var trace = cd0.trace;
if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
// box half width
var bdPos = t.dPos * groupFraction * (1 - fullLayout.boxgroupgap) / (group ? numBoxes : 1);

// position coordinate delta
var dPos = t.dPos;
// box half width;
var bdPos;
// box center offset
var bPos = group ? 2 * t.dPos * (-0.5 + (t.num + 0.5) / numBoxes) * groupFraction : 0;
var bPos;

if(trace.width) {
bdPos = dPos;
bPos = 0;
} else {
bdPos = dPos * groupFraction * groupGapFraction / (group ? numBoxes : 1);
bPos = group ? 2 * dPos * (-0.5 + (t.num + 0.5) / numBoxes) * groupFraction : 0;
}

// whisker width
var wdPos = bdPos * trace.whiskerwidth;

Expand Down
9 changes: 9 additions & 0 deletions src/traces/violin/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,15 @@ module.exports = {
'right (left) for vertical violins and above (below) for horizontal violins.'
].join(' ')
}),

width: extendFlat({}, boxAttrs.width, {
description: [
'Sets the width of the violin in data coordinates.',
'If *0* (default value) the width is automatically selected based on the positions',
'of other violin traces in the same subplot.',
].join(' ')
}),

marker: boxAttrs.marker,
text: boxAttrs.text,

Expand Down
35 changes: 22 additions & 13 deletions src/traces/violin/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,10 @@ module.exports = function calc(gd, trace) {
trace[trace.orientation === 'h' ? 'xaxis' : 'yaxis']
);

var violinScaleGroupStats = fullLayout._violinScaleGroupStats;
var scaleGroup = trace.scalegroup;
var groupStats = violinScaleGroupStats[scaleGroup];
if(!groupStats) {
groupStats = violinScaleGroupStats[scaleGroup] = {
maxWidth: 0,
maxCount: 0
};
}

var spanMin = Infinity;
var spanMax = -Infinity;
var maxKDE = 0;
var maxCount = 0;

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

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

groupStats.maxCount = Math.max(groupStats.maxCount, vals.length);

maxCount = Math.max(maxCount, vals.length);
spanMin = Math.min(spanMin, span[0]);
spanMax = Math.max(spanMax, span[1]);
}

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

if(trace.width) {
cd[0].t.maxKDE = maxKDE;
} else {
var violinScaleGroupStats = fullLayout._violinScaleGroupStats;
var scaleGroup = trace.scalegroup;
var groupStats = violinScaleGroupStats[scaleGroup];

if(groupStats) {
groupStats.maxKDE = Math.max(groupStats.maxKDE, maxKDE);
groupStats.maxCount = Math.max(groupStats.maxCount, maxCount);
} else {
violinScaleGroupStats[scaleGroup] = {
maxKDE: maxKDE,
maxCount: maxCount
};
}
}

cd[0].t.labels.kde = Lib._(gd, 'kde:');

return cd;
Expand Down
8 changes: 6 additions & 2 deletions src/traces/violin/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,14 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
if(traceOut.visible === false) return;

coerce('bandwidth');
coerce('scalegroup', traceOut.name);
coerce('scalemode');
coerce('side');

var width = coerce('width');
if(!width) {
coerce('scalegroup', traceOut.name);
coerce('scalemode');
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anything break if we 🔪 the else entirely? Unnecessary attributes should simply not be included in traceOut at all.

Copy link
Contributor Author

@Kully Kully Nov 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the violins are not drawn properly: the outlines are filled with black ink


var span = coerce('span');
var spanmodeDflt;
if(Array.isArray(span)) spanmodeDflt = 'manual';
Expand Down
9 changes: 6 additions & 3 deletions src/traces/violin/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,22 @@ module.exports = {
'If *group*, the violins are plotted next to one another',
'centered around the shared location.',
'If *overlay*, the violins are plotted over one another,',
'you might need to set *opacity* to see them multiple violins.'
'you might need to set *opacity* to see them multiple violins.',
'Has no effect on traces that have *width* set.'
].join(' ')
}),
violingap: extendFlat({}, boxLayoutAttrs.boxgap, {
description: [
'Sets the gap (in plot fraction) between violins of',
'adjacent location coordinates.'
'adjacent location coordinates.',
'Has no effect on traces that have *width* set.'
].join(' ')
}),
violingroupgap: extendFlat({}, boxLayoutAttrs.boxgroupgap, {
description: [
'Sets the gap (in plot fraction) between violins of',
'the same location coordinate.'
'the same location coordinate.',
'Has no effect on traces that have *width* set.'
].join(' ')
})
};
47 changes: 32 additions & 15 deletions src/traces/violin/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
var fullLayout = gd._fullLayout;
var xa = plotinfo.xaxis;
var ya = plotinfo.yaxis;
var numViolins = fullLayout._numViolins;
var group = (fullLayout.violinmode === 'group' && numViolins > 1);
var groupFraction = 1 - fullLayout.violingap;
var groupGapFraction = 1 - fullLayout.violingroupgap;

function makePath(pts) {
var segments = linePoints(pts, {
Expand All @@ -39,16 +43,30 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
var t = cd0.t;
var trace = cd0.trace;
if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
var numViolins = fullLayout._numViolins;
var group = (fullLayout.violinmode === 'group' && numViolins > 1);
var groupFraction = 1 - fullLayout.violingap;

// position coordinate delta
var dPos = t.dPos;
// violin max half width
var bdPos = t.bdPos = t.dPos * groupFraction * (1 - fullLayout.violingroupgap) / (group ? numViolins : 1);
var bdPos;
// violin center offset
var bPos = t.bPos = group ? 2 * t.dPos * (-0.5 + (t.num + 0.5) / numViolins) * groupFraction : 0;
var bPos;
// half-width within which to accept hover for this violin
// always split the distance to the closest violin
t.wHover = t.dPos * (group ? groupFraction / numViolins : 1);
var wHover;

if(trace.width) {
bdPos = dPos;
bPos = 0;
wHover = dPos;
} else {
bdPos = dPos * groupFraction * groupGapFraction / (group ? numViolins : 1);
bPos = group ? 2 * dPos * (-0.5 + (t.num + 0.5) / numViolins) * groupFraction : 0;
wHover = dPos * (group ? groupFraction / numViolins : 1);
}

t.bdPos = bdPos;
t.bPos = bPos;
t.wHover = wHover;

if(trace.visible !== true || t.empty) {
plotGroup.remove();
Expand All @@ -60,7 +78,6 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
var hasBothSides = trace.side === 'both';
var hasPositiveSide = hasBothSides || trace.side === 'positive';
var hasNegativeSide = hasBothSides || trace.side === 'negative';
var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup];

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

Expand All @@ -76,15 +93,15 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
var len = density.length;
var posCenter = d.pos + bPos;
var posCenterPx = posAxis.c2p(posCenter);
var scale;

switch(trace.scalemode) {
case 'width':
scale = groupStats.maxWidth / bdPos;
break;
case 'count':
scale = (groupStats.maxWidth / bdPos) * (groupStats.maxCount / d.pts.length);
break;
var scale;
if(trace.width) {
scale = t.maxKDE / bdPos;
} else {
var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup];
scale = trace.scalemode === 'count' ?
(groupStats.maxKDE / bdPos) * (groupStats.maxCount / d.pts.length) :
groupStats.maxKDE / bdPos;
}

var pathPos, pathNeg, path;
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
44 changes: 44 additions & 0 deletions test/image/mocks/violin_box_multiple_widths.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"data": [{
"type": "violin",
"width": 0.4,
"name": "width: 0.4",
"x": [0, 5, 7, 8],
"side": "positive",
"line": {
"color": "black"
},
"fillcolor": "#8dd3c7",
"opacity": 0.6,
"y0": 0.0
}, {
"type": "violin",
"name": "auto",
"x": [0, 5, 7, 8],
"side": "positive",
"line": {
"color": "black"
},
"fillcolor": "#d3c78d",
"opacity": 0.6,
"y0": 0.1
}, {
"type": "box",
"width": 0.6,
"name": "width: 0.6",
"x": [0, 5, 7, 8],
"side": "positive",
"line": {
"color": "black"
},
"fillcolor": "#c78dd3",
"opacity": 0.6,
"y0": 0.2
}],
"layout": {
"title": "Joyplot - Violin with multiple widths",
"legend": {"x": 0},
"xaxis": {"zeroline": false},
"yaxis": {"dtick": 0.1, "gridcolor": "black"}
}
}
Loading