-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Feature: draw percentiles in violinplot #14107
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
Merged
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
e1985d7
Fix issue 10788
sasoripathos 2279b16
Fix style
sasoripathos 19bb0c7
Add missing import statement
sasoripathos b7fdc9b
Merge remote-tracking branch 'upstream/master'
sasoripathos 109f252
Merge remote-tracking branch 'upstream/master'
sasoripathos b2ca609
Merge remote-tracking branch 'upstream/master'
sasoripathos 2c5a873
Merge remote-tracking branch 'upstream/master'
sasoripathos cb0a64c
Remove changes
sasoripathos 67664d5
Code for feature 8532
sasoripathos ea5dd1c
Run boilerplate
sasoripathos 5552d3d
Fix code style
sasoripathos 3fd9152
Remove unused test images
sasoripathos 80d3fd8
Update pyplot.py
sasoripathos f78fcac
Update pyplot.py
sasoripathos dbda148
Run boilerplate
sasoripathos fb32794
Remove extra image-compare tests, update documentation
sasoripathos b8ae7bd
Merge remote-tracking branch 'upstream/master'
sasoripathos 0bb79ec
Merge branch 'master' into feature8532
sasoripathos 3d46d7f
Merge remote-tracking branch 'upstream/master' into feature8532
sasoripathos a3912a9
Use np.quantile instead of percentile
sasoripathos 6e50219
Merge remote-tracking branch 'upstream/master'
sasoripathos ee5cd08
Update test images for violinplot
sasoripathos 999a424
Merge branch 'master' into feature8532
sasoripathos eafd766
Revert "Update test images for violinplot"
sasoripathos e48658f
Use np.percentile implement quantile
sasoripathos 8cbafae
Merge remote-tracking branch 'upstream/master'
sasoripathos 6f3e2a0
Merge branch 'master' into feature8532
sasoripathos 7a4777b
Update api from percentiles to quantiles
sasoripathos 5d80c50
Merge branch 'feature8532' of https://github.com/sasoripathos/matplot…
sasoripathos 6afe73a
Update violinplot call in test cases
sasoripathos 54f955a
Update documentations from percentiles to quantiles
sasoripathos a0c3073
Update docstring and backward campatibility
sasoripathos c586932
Remove unnecessary api change doc
sasoripathos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file modified
BIN
+370 Bytes
(100%)
lib/matplotlib/tests/baseline_images/test_axes/violinplot_horiz_showall.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+1.13 KB
(100%)
lib/matplotlib/tests/baseline_images/test_axes/violinplot_vert_showall.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is a breaking API change. I'm afraid, there's no way around that, but it at least needs an API change note.
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.
I am confused. If user directly use this function as API, my code only extends the results that user can get. If this extension is considered as a "break", then I don't think it is necessary to make qunatiles key optional. According to current code, the option, whether to add a Linecollection, is decided by violin().
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.
If user never directly use this function, then it is only called by violinplot(). Should this be counted as a API though?
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.
Matplotlib is extensively used out in the wild. By experience, for every remote but possible way of using the official API, there's someone out there, who actually has code with it. Therefore, we've become quite sensitive to possible API breaks. They will pop up as bug reports sooner or later. So we have to at least document the changes.
Tuple unpacking of the result will break with the change:
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.
I agree to have a document. But I feel it is odd to make quantiles optional since the current code in violin_stats doesn't care whether those value will be drawn out.
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.
I'm sorry, I misread the documentation here. It's a list of dictionaries with one dictionary per input dataset. Adding keys to these dicts is not breaking the API. So the above API change note is not necessary and should be removed again. Sorry for the noise.