-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Added donut chart functionality via new axes.pie parameter #10944
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
Conversation
Doughnut Charts | ||
----------------------------------- | ||
|
||
A doughnut chart is essentially a pie chart with an area of the center cut out. It is one of the most aesthetically pleasing to look at charts that there is. In Matplotlib, it is apparent there there is no consolidated method of drawing a doughnut chart. With our changes we solve this problem. |
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.
Let's not make judgment calls about the esthetics of donuts. Also be consistent doughnut/donut.
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.
Donuts need to be gustatory, not aestetic!
I am not conviced at all that such a complex new API is needed, as opposed to documenting something like
|
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.
Per #10944 (comment).
Feel free to dismiss in case of consensus amoung other devs for adding the feature.
Wasn't there a donut example somewhere in the gallery? I just found this one which misses the point of using the If there is none, maybe take something like this (adapted from my SO answer and add it to the gallery:
|
Thinking about it, maybe the existing nested_pie example should be kept and modified to have a hole in the center? Then the text could just be adapted and contain the word "donut chart", such that the example is found in search engines under that name? |
Hey all, Thanks for the quick feedback of my PR. Just thought I should give it some more context. Originally, I had noticed confusion from some online posters trying to make donut charts. Solutions varied greatly, and involved anything from dropping a circle in the middle of pie charts to drawing multiple nested pies as described in the example code posted above. Basically, because 'donut' was the commonly searched term for these types of charts, I wanted to allow users to use this word as a flag to easily create them, which could easily be done using axes.pie. Interesting to see that this may just be a documentation issue, and also that wedgeprops actually handles adjusting the width property. Anyways, thanks again for the feedback, and I apologize if it seemed my PR was misguided. |
No worries for the PR. If there's a reasonable feature that you can't figure out how to do, but can actually be done easily, that still qualifies as a valid documentation issue (or at least "documentation improvement request"). We just want to try to keep the actual matplotlib API at a reasonable size... |
@story645 I think the agreement is to not make an API change but just improve the docs? (a new feature doesn't really qualify as a API change either, I think.) |
But it's got a new kwarg? I think that's an API change...unless the kwarg is gonna go away? |
I think both @ImportanceOfBeingErnest and myself believe (well, I'm certain for myself :p) that the new kwarg should not be added. |
I don't think whether or not I personally like or dislike this matters in any ways; so rather to give some argument here: This PR adds some kind of donut-API inside the pie-API. Such nested APIs are difficult to maintain and difficult to grasp for users. There are already some similar constructs within matplotlib, the "worst" being the This donut-API is a virtual thing. There is no Donut object anywhere, so it essentially does something which can be achieved with existing means already. In that sense it seems more useful to promote the existing capabilities instead of adding a new path of achiving the same thing.
Which one would you prefer? |
Of course your opinion does matter :) |
I gave it a shot at #10953 . |
We also have a (very domain specific) variant on this in the examples already (https://matplotlib.org/gallery/specialty_plots/leftventricle_bulleye.html?highlight=bullseye) @AlexCav Even though we are not going to merge this thank you for your work and identifying a use case we can easily do but was not well documented. Hopefully this will not dissuade you from contributing again in the future! |
PR Summary
Added a new optional parameter 'donut' to axes.pie in order to format pie charts into a donut-like shape. This parameter requires a dictionary, where user can specify the donut shape's width, draw text in the center of the chart, and specify breakpoints in the original data set to create multiple donut chart layers.
See doc/users/next_whats_new/donut_chart.rst for feature documentation.
Code changes made in _axes.py, with image comparison tests in test_axes.py.
PR Checklist