Implement notched box plots, closes #2286 #1
Conversation
There was a problem hiding this comment.
@krassowski this is great, thanks for adding the feature! I just made a few comments, feel free to PR to the main repo after fixing. We may have to wait until Monday to see if anyone else feels strongly about the notchwidth convention.
src/traces/box/defaults.js
Outdated
| @@ -29,6 +29,9 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) { | |||
| coerce('whiskerwidth'); | |||
| coerce('boxmean'); | |||
|
|
|||
| coerce('notched'); | |||
| coerce('notchwidth'); | |||
There was a problem hiding this comment.
We should only coerce notchwidth if notched is true.
src/traces/box/attributes.js
Outdated
| description: [ | ||
| 'Sets the width of the notches relative to', | ||
| 'the box\' width.', | ||
| 'For example, with 1, the whiskers are as wide as the box(es).' |
There was a problem hiding this comment.
It looks like Matlab and ggplot have opposite conventions about this... if I'm reading it right this is the ggplot version. I would have probably gone for the Matlab version, where notchwidth=0 is nothing taken away, and notchwidth=0.5 is notches that meet in the middle (actually I might have called that 1 instead... but we don't want to introduce yet ANOTHER convention). I guess I don't have that strong an opinion about it though, @etpinard or @cpsievert do you care which we use?
There was a problem hiding this comment.
Good point; I will wait for others to express their opinion. Just to have it here: the behavior (and description) of notchwidth is based on another, already existing attribute - whiskerwidth.
| @@ -0,0 +1,287 @@ | |||
| { | |||
| "data":[ | |||
There was a problem hiding this comment.
Very nice test images! re: the labels getting cut off - that'll get fixed by plotly#2243 but here we might as well just give it a larger left margin so they fit.
src/traces/box/plot.js
Outdated
| 'M' + q1 + ',' + pos0 + 'V' + pos1 + 'H' + q3 + 'V' + pos0 + 'Z' + // box | ||
| 'M' + m + ',' + posm0 + 'V' + posm1 + // median line | ||
| 'M' + q1 + ',' + pos0 + 'V' + pos1 + // left edge | ||
| (((notched === true)) ? 'H' + ln + 'L' + m + ',' + posm1 + 'L' + un + ',' + pos1 : '') + // top notched edge |
There was a problem hiding this comment.
That's a lot of (())... I think it can be just (notched ? ... : '')
Unbind VAO in gl-surface3d
I deliberately removed `"notched":true` in one of test box traces so it is (indirectly) tested
presumably for better consistency with Matlab
I added a boolean option "notched" (default false) and a numerical "notchwidth" (default 0.5).
The options define if the box trace should be notched or not, and if yes, determine the width of the notch (proportional to the width of the box).
I modified the svg path-generation code, utilizing conditional (ternary) operators so the common parts of the box (top & bottom edges for vertical, left & right edges for horizontal) are shared.
I added two test baselines:
box_notchedandbox_horz_notched.