-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
update nested-pie example with donut #10953
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
ax.pie(vals.flatten(), radius=1-size, colors=cin, | ||
wedgeprops=dict(width=size, edgecolor='w')) | ||
|
||
ax.set(aspect="equal", title='Pie plot with `ax.pie`') |
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 of #10914 aspect="equal" is not needed anymore.
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 thought this could get into the docs already right now, not with the next feature release where aspect="equal" is automatically set.
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.
Agreed. Can we take the aspect out with an additional PR after backporting?
|
||
cm = plt.get_cmap("tab20c") | ||
cout = cm(np.arange(3)*4) | ||
cin = cm(np.array([1, 2, 5, 6, 9, 10])) |
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.
Can you please make the variables more telling? i.e. outer_colors
, inner_colors
, cmap
.
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.
Agreed.
height=np.zeros_like(left_outer) + 3) | ||
ax.bar(x=valsleft.flatten(), | ||
width=valsnorm.flatten(), bottom=1-2*size, height=size, | ||
color=cin, edgecolor='w', linewidth=1, align="edge") | ||
|
||
ax.set(title="Pie plot with `ax.bar` and polar coordinates") |
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.
Out of curiosity: Do we have general recommendations for ax.set(prop=...)
vs. ax.set_prop(...)
? I'm using the latter one for single properties.
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 kept that from the previous example. I do not know anything about recommended style here.
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.
fwiw I really like .set(foo=bar)
for everything.
Could you add labels as in your original example and/or a legend, so that this looks more like a real-world usecase. Also, showing how to label your data is an important piece of information that should be conveyed in the example. The nested donuts are already a more complex example. Maybe we should additionally add a simple donut plot, which could then include part of the labelling information. |
There is the basic pie example and the pie demo2 both showing how to add labels. Are you sure this is needed? The main point of this example was and is to show (a) nesting, (b) the two paths to get a pie (through pie and polar bars). I would not clutter it with additional labels or legends. One thing we apparently do not have is an example of a pie chart with a legend. I could add that. Should that also show a donut then? |
How hard would it be to put the nested pie charts into a function that users could copy-paste as a base for customized helper functions? |
Specifically, functions that look like those described at https://matplotlib.org/tutorials/introductory/usage.html#coding-styles |
Help! 🆘 I created the new example, which now looks like this. because apparently the figure is created at the point where it ... (I don't know actually); but in any case I want it to be created at the bottom of the page of couse, when it got all its content. |
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.
Great example. I'm hungry now ... 😄
# Now it's time for the pie. Starting with a pie recipe, we create the data | ||
# and a list of labels from it. | ||
# | ||
# We can `provide a function to the ``autopct`` argument, which will expand |
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.
extra backtick before "provide"
|
||
pie = ax.pie(data, autopct=lambda pct: func(pct, data), | ||
textprops=dict(color="w")) | ||
|
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.
wedges, texts, autotexts = ax.pie(...
or on a separate line wedges, texts, autotexts = pie
# dictionaries of common properties, which we can later pass as keyword | ||
# argument. We then iterate over all wedges and for each | ||
# | ||
# * calcualte the angle of the wedge's center |
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.
calculate
donut = ax2.pie(data, radius=1, wedgeprops=dict(width=0.5), startangle=-40) | ||
|
||
bbox_props = dict(boxstyle="square,pad=0.3", fc="w", ec="k", lw=0.72) | ||
kw = dict(xycoords='data',textcoords='data',arrowprops=dict(arrowstyle="-"), |
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.
space after comma
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.
No need to go through the PEP8 stuff; because I cannot build the docs locally, I need to misuse the tests on the server for now. I will try to squash at the end, but this will not be the last commit anyways.
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.
you can still check for PEP8 with something like pytest -m pep8 --pep8 path_to_example.py
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.
# | ||
# * calcualte the angle of the wedge's center | ||
# * from that obtain the coordinates of the point at that angle on the | ||
# circonference |
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.
circumference
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.
Sir! Conferences always turn in circles! 😆
# of the circle the point lies. | ||
# * update the connection style with the obtained angle to have the annotation | ||
# arrow point outwards from the donut | ||
# * finally create the annotation with all the previously determined parameters |
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.
period after every sentence of the bullet list.
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 not a native speaker, but shouldn't sentences have a subject to be sentences? I'll go with a comma instead.
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 not a native speaker either.
Apparently this construct is called "vertical lists punctuated as a sentence." The subject is "we" in the stem. There seem to be different opinions what to use. When searching you'll find everything from colon, semicolon, comma to nothing. So do as you like.
Essentially the nested pie chart example can be copied into a function as it is. However I suppose the user needs to create one such function per given number of data points due to the colors being manually chosen from the colormap. Creating the logic to choose n colors with m shades of that color for a general purpose plotter goes a bit beyond the scope of this, doesn't it? Or maybe I misunderstood the question, @tacaswell ? |
plt.setp((ax, ax2), aspect="equal") | ||
|
||
|
||
############################################################################### |
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 think this divider is why the plot isn't rendering fully
As @phobson suggested,
I removed the divider, but then the comments are real comments to the code, not text within the document. See result here. I would rather keep the text as text with links etc. than having it as commented code. I now opened an issue about that at sphinx-gallery to see if anything can be done about it. If not, the example would probably need to be produced in two single figures. |
Since the issue of creating the figure only at the end of the code cannot be solved currently as seen in sphinx gallery #363, I used two example figures. This is fine, except that the gallery thumbnail will only show the first of those. Now, I tried squashing the 6 commits into one; now I ended up with 8 commits?? (Is there anything missing in rewriting-commit-history that would cause this? But I think apart from me fighting with git, this is good to go now? |
There is a Sphinx gallery command you can put in comments to set the thumbnail figure. I used it in the constrained layout tutorial if you’d like an example ( on phone so hard to look up directly). EDIT: To fix the git tangle suggest you move your modified file somewhere else, do
and then copy the file back, commit and push. This’ll leave you w one commit against master. Not such a good method if you have many changes but if just one file it works really well. |
Ps I don’t usually merge commit. I do |
This is crazy. There must be something like |
You shouldn't really have conflicts (no one else has edited this file). Strongly suggest you do what I origially suggested first and reset your branch to upstream/master and start over if you are getting conflicts: it sounds like giving up, but it really is the easiest way to recover from a mess up. Of course you need not type in 9 digit numbers (though cutting and pasting from |
I do:
|
fe4ccd5
to
511cde7
Compare
öahm... don't know if that was a good idea?! @phobson |
It’s because you did the merge. Just copy the files aside and reset hard and you’ll be fine. |
511cde7
to
277b3b0
Compare
I think I finally made it. Concerning images it's not that one is better than the other; it's just it would have been nice to have them both as thumbnail. That would require to produce come static image, which is probably not desireable. |
Great - Git can be a bit of a PITA if you hit a glitch in your workflow. Thanks for sticking with it. I’ll merge once CI passes. These are great examples! Thanks... |
ang = (p.theta2 - p.theta1)/2. + p.theta1 | ||
y = np.sin(np.deg2rad(ang)) | ||
x = np.cos(np.deg2rad(ang)) | ||
horizontalalignment = ["", "left", "right"][int(np.sign(x))] |
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.
very minor nitpick: I think this reads better as e.g. {-1: "right", 1: "right"}[np.sign(x)]
. Can be done in a separate PR if you prefer this to go in first, I agree it's a very nice example overall.
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 totally agree; that is not very understandable.
If it's ok with you I would then just do a second commit here without trying to squash merge and unwillingly inviting some other people's PRs to the party. (Still not sure what I did wrong for that to happen.)
Backport PR #10953 on branch v2.2.2-doc
PR Summary
Update nested_pie example
The PR #10944 attempted to provide a new argument to
pie
whose intention was to implement a donut chart API to thepie
function. While this is not desireable (see arguments in discussion of that PR), what can be taken from this is that users are not aware of the easy options to produce donut charts in matplotlib.This PR changes the existing nested-pie example from having several rings, but the inner-most filled to the center, to actually have a hole in them. The concepts of the codes remain; yet the result is more towards applications. It should also be more straight forward to adapt from the new example to a filled version (just leaving out the
width
orbottom
parameter than it was previously to imagine that this example would also be useful to produce a donut chart.Further this PR unifies the two examples given such that they produce the same plot through two different approaches. The produced chart then looks like this for both example cases:
Create example for labeling pies
In addition this PR now incorporates a new example showing how to label pies with a legend as well as with annotations.
PR Checklist