Fix width of box plots with log-scale axis #4283
Conversation
| @@ -213,6 +212,7 @@ function setPositionOffset(traceType, gd, boxList, posAxis) { | |||
| padded: padded, | |||
| vpadminus: vpadminus, | |||
| vpadplus: vpadplus, | |||
| vpadLinearized: true, | |||
There was a problem hiding this comment.
So, could you explain why we need to pad in linear space (l) for log axes as opposed to in calc space (c)?
Padding in calc space on log axes seems to work ok in other scenarios like scatter-marker traces.
There was a problem hiding this comment.
I think paddings in scatter-marker traces are not based on values (vpad), but pixels (ppad). So the padding is not affected by axes types.
Another example that uses values as margins is box traces, but box traces should be based on c values rather than l values.
| 100, | ||
| 100 | ||
| ], | ||
| "type": "box" |
There was a problem hiding this comment.
Have you tried plotting these samples as type: 'violin' traces?
Violin and box traces share a lot of the same code, so it might be nice to fix the log-position problems for violin traces in the same go.
@s417-lama please let us know if you're not interested in working on a fix for violin traces. We should be able to find some time to work on it before releasing v1.51.0.
There was a problem hiding this comment.
Oh, I didn't consider violin traces.
I would be glad for you to work on violin plots. I appreciate it.
There was a problem hiding this comment.
Great. Thanks!! The fix for violins should be very similar than the one for boxes.
| @@ -0,0 +1,94 @@ | |||
| { | |||
There was a problem hiding this comment.
Thanks very much for taking a shot a violin traces on log axes!
There seem to be a small problem with the position of the violin "box":
the tall line at the right of the box should be drawn where the shading from the one-side violin ends.
There was a problem hiding this comment.
Actually this has nothing to do with your PR, all vertical one-sided violins have their box line shifted in the wrong direction e.g.:
https://codepen.io/etpinard/pen/RwwZZzj?editors=1010
No need to address this here. I'll make a PR in the next few minutes fixing that.
|
Awesome work @s417-lama - this is one of the best community PRs we've received in recent memory. I was hoping to find a slightly less convoluted way to dance between Thank you very much. Merging. This will be released in |
Fixes an issue of box plots that box width is also log-scaled with log axis (fixes #3508).
I changed the setting of box widths from raw-value-based one to linearized-value-based one.
This PR is pre-reviewed in s417-lama#1 by @etpinard .