Changed pie charts default shape to circle and added tests #10914
Conversation
lib/matplotlib/axes/_axes.py
Outdated
| @@ -2670,8 +2670,9 @@ def pie(self, x, explode=None, labels=None, colors=None, | |||
| Notes | |||
| ----- | |||
| The pie chart will probably look best if the figure and axes are | |||
| square, or the Axes aspect is equal. | |||
| square, or the Axes aspect is equal, this is the default setting. | |||
There was a problem hiding this comment.
Its not the default setting, its the only setting. Suggest:
This method sets the aspect ratio fo the axis to ``equal``.
There was a problem hiding this comment.
I was trying to say that is the default setting for pie, but I think what you said is more clear, thanks
lib/matplotlib/axes/_axes.py
Outdated
| @@ -2672,6 +2672,9 @@ def pie(self, x, explode=None, labels=None, colors=None, | |||
| The pie chart will probably look best if the figure and axes are | |||
| square, or the Axes aspect is equal. | |||
| """ | |||
| # This method sets the aspect ratio of the axis to "equal". | |||
There was a problem hiding this comment.
This should not be a comment but part of the docstring. Please rework the note in the doctring to reflect the new behavior.
There was a problem hiding this comment.
heh, yeah, this should be the Note part of the docstring, not a comment.
lib/matplotlib/tests/test_axes.py
Outdated
| labels = 'Frogs', 'Hogs', 'Dogs', 'Logs' | ||
| sizes = [15, 30, 45, 10] | ||
| colors = ['yellowgreen', 'gold', 'lightskyblue', 'lightcoral'] | ||
| explode = (0, 0.1, 0, 0) # only "explode" the 2nd slice (i.e. 'Hogs')z |
lib/matplotlib/tests/test_axes.py
Outdated
|
|
||
|
|
||
| @image_comparison(baseline_images=['pie_default'], extensions=['png']) | ||
| def test_pie_default_2(): |
There was a problem hiding this comment.
Why the two test? Is it for ax.pie vs pyplot.pie? I'm not sure if we need both. pyplot.pie is just a very thin wrapper around Axes.pie. Maybe someone with more experience on matplotlib tests can comment on this.
If both should stay, please name them semantically, e.g. ..._axes, ..._pyplot instead of just numbering them.
There was a problem hiding this comment.
yes it's axes.pie vs pyplot.pie, I added 2 tests because sometimes it might pass the pyplot.pie test and fail the axes.pie test (this is the case if you add axis("image") to pyplot.pie) , but I am not sure if its always the case that if axes.pie passes then pyplot.pie must also pass.
edit: Thanks, looks like only one is needed
|
Comments in #10789 mention a what's new entry. Is this necessary or is it a too small change? Also, should the docstring state how to revert to the old behaviour? |
|
Yes an entry in It couldn't hurt to say: Thanks @ImportanceOfBeingErnest for noting that. |
|
add in both doc/users/next-whats-new/ and also the docstring for pie ? |
|
See the other whats-new entries for examples of what should go in there. The not about how to set aspect ratio should be in the docstring |
|
Thanks @jerrylui803 and congratulations on your first Matplotlib contribution 🎉 ! Hopefully we will hear from you again! |
|
housekeeping: I squash-merged this to reduce code churn. |
github.com//issues/10789
As seen in the issue link above, a simple way to generate a pie graph is using plt.pie, and the second method to generate a pie graph is by using plt.subplots. Both of these approaches creates the pie graph through _axes.py by using an Axes instance which calls its pie method; the additional line to add calls the set_aspect function, which sets the aspect of the axis scaling, with ‘equals’ as parameters and this results in a default circular pie graph with an equal aspect ratio as wanted.
Some test cases are also added.
PR Summary
PR Checklist