Adding box plots drawn on std-deviation instead of quartiles #6698
Conversation
Creating sizemode and sdmultiple to allow plots with any standard deviation, and a simpler cleaner codebase. use 'showwhisker' insteadk of 'whiskerdisable' adding .png to allow unit test to pass hover annotation labels number of std deviations chosen
|
Hi @alexcjohnson thank you for the feedback.
|
tidy code Co-authored-by: Alex Johnson <[email protected]>
…otly.js into dev-box-sigma-boxes
Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>
simplify sizemode usage
|
Hey @alexcjohnson. The baselines for Violin and Candlesticks were failing, because inside box/plot.js "trace.showwhiskers" was undefined. I see your earlier comment and started using !== false. Thanks! The remaining failing test is [webgl-jasmine], but seems unrelated to these changes |
archmoj
added
feature
|
|
||
| sdmultiple: { | ||
| valType: 'number', | ||
| min: 0, |
There was a problem hiding this comment.
Shouldn't the minimum be one instead of zero?
There was a problem hiding this comment.
what if you want 0.5σ? Might be weird but I don't see a reason to prohibit it.
CONTRIBUTING.md
Outdated
| #### Step 6b: Create a mock to test new features | ||
| Create a new JSON inside | ||
| `test\image\mocks\` | ||
| which you'll then be able to search from the test dashboard |
There was a problem hiding this comment.
In several cases adding new image tests are not necessary.
So this step should be optional and it could be part of Step 6.
You may revert changes in CONTRIBUTING.md and possibly submit a separate PR to suggest improvements as well.
Thank you!
| @@ -286,7 +286,9 @@ module.exports = function calc(gd, trace) { | |||
| q1: _(gd, 'q1:'), | |||
| q3: _(gd, 'q3:'), | |||
| max: _(gd, 'max:'), | |||
| mean: trace.boxmean === 'sd' ? _(gd, 'mean ± σ:') : _(gd, 'mean:'), | |||
| mean: (trace.boxmean === 'sd') || (trace.sizemode === 'sd') ? | |||
| _(gd, 'mean ± σ:').replace('σ', trace.sdmultiple === 1 ? 'σ' : (trace.sdmultiple + 'σ')) : // displaying mean +- Nσ whilst supporting translations | |||
There was a problem hiding this comment.
This line is getting too long.
How about adding agetMean(gd, trace) and move & revise this logic there?
There was a problem hiding this comment.
Putting the code into a function would make the code less readable - if readability is the concern here.
There was a problem hiding this comment.
I agree - if it's only going to be used here, extracting to a function just means it takes another jump to read and understand the code. @archmoj if your concern is just line length we can break this onto more lines. Or leave it for now and sometime add prettier to our toolchain like we have in various other projects 😉
|
I've taken action on the comments |
|
Wow, thanks everyone! |
To help with this issue
#6697
I've made below changes, I've tried to keep the change to the minimum. Resulting image of new plot is in the issue
I also touched CONTRIBUTING.md because this instructions opens an empty webpage and it's really not obvious how to use it