Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Add side option to violinplot#27815
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join uson gitter for real-time discussion.
For details on testing, writing docs, and our review process, please seethe developer guide
We strive to be a welcoming and open project. Please follow ourCode of Conduct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fairly minor things:
Testing standard practices (using new style, random seed)
- You were matching local examples, but we do have preferences for new tests
Using helper function for validation.
As this is still in draft, didn't look too hard at the docs changes yet, but figured I'd point to some of our tooling early on.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Can I fix the failed check without creating a new PR? (I changed the test image half-way through) |
Yes, you can (and in fact opening a new PR alone would not even prevent the flag): We have a guide on interactive rebasing here: https://matplotlib.org/devdocs/devel/development_workflow.html#rewrite-commit-history At minimum you'd need to squash together the two commits that added images. The easiest way to do that is probably to just squash all the commits down to one commit (its a small enough change for that) If you do not want to mess with it yourself, we can do it as well. |
7a252c3
todc97efb
CompareI tried myself, failed a little, but I think I managed to squash the commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As one can see in the image, there's a gap where extrema line and vertical line meet. We could investigate whether hard-codingcapstyle="projecting"
makes sense for extrema lines of half violins.
https://matplotlib.org/devdocs/gallery/lines_bars_and_markers/capstyle.html#capstyle
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
cd8ef8d
to50d2950
CompareUh oh!
There was an error while loading.Please reload this page.
50d2950
to2782e0e
Compareadd test for side option in violinplotadd whats new for side in violinplotadd violin side option to _axes.pyi and pyplot.pyuse side option in violinplot also for linesadd side option to violinplot exampleuse newer style and correct seed in violin testupdate imagechange option naminguse proper syntax for code in violin docsCo-authored-by: Kyle Sunden <git@ksunden.space>add changed violinplots (capstyle=projecting)hardcode capstyle only for left-right violinsfix typo
2782e0e
to0adab7b
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM. Thanks for the patience to go through all the review cycles!
Thanks to you! This was very pleasant and I will definitely contribute to matplotlib in the future now that I know how you do things :) |
Congrats on your merged PR! Thanks |
Uh oh!
There was an error while loading.Please reload this page.
PR summary
This PR adds a
side
parameter to the violin plot which allows to plot only one half of the violin.I added an image comparison for vertical and horizontal violins and two subplots in the gallery example.
closes#27812
PR checklist