-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add option to rotate labels in a pie chart (#2304) #8217
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
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.
Thanks for the thorough and well-tested PR!
lib/matplotlib/axes/_axes.py
Outdated
@@ -2640,11 +2644,15 @@ def get_next_color(): | |||
xt = x + labeldistance * radius * math.cos(thetam) | |||
yt = y + labeldistance * radius * math.sin(thetam) | |||
label_alignment = xt > 0 and 'left' or 'right' | |||
label_rotation = 'horizontal' | |||
if labelrotate: | |||
label_rotation = thetamd + (0 if xt > 0 else 180) |
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.
Instead of using thetamd
I think it would be better to just use np.rad2deg(thetam)
This looks really good, thanks for the PR! I think |
Just noticed vertical alignment needs to be updated as well, it should be fixed now. |
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 looks good now, thanks for the work! I will merge if/when travis and appveyor tests pass.
It looks like there's a PEP8 violation: https://travis-ci.org/matplotlib/matplotlib/jobs/208805540#L1676 - other than that the travis tests seem fine. |
Thanks!
|
Ugh, sorry, I should have noticed that - thanks for pointing it out. I'll fix that myself and merge this pull request when I've done that. |
@@ -3208,7 +3208,7 @@ def phase_spectrum(x, Fs=None, Fc=None, window=None, pad_to=None, sides=None, | |||
def pie(x, explode=None, labels=None, colors=None, autopct=None, | |||
pctdistance=0.6, shadow=False, labeldistance=1.1, startangle=None, | |||
radius=None, counterclock=True, wedgeprops=None, textprops=None, | |||
center=(0, 0), frame=False, hold=None, data=None): |
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.
Isn't that file autogenerated?
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.
Yes, the changes on this file are generated by boilerplate.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.
As long as you haven't manually updated this file, it's all good :)
@@ -2475,7 +2475,7 @@ def pie(self, x, explode=None, labels=None, colors=None, | |||
autopct=None, pctdistance=0.6, shadow=False, labeldistance=1.1, | |||
startangle=None, radius=None, counterclock=True, | |||
wedgeprops=None, textprops=None, center=(0, 0), | |||
frame=False): | |||
frame=False, rotatelabels=False): | |||
r""" |
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.
Do you mind converting this docstring into numpydoc? It will make it much more readable.
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 would be happy to, do you want me to create a separate issue for that?
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.
Yes, let's do this. I am going to merge this PR straight away, as everything is fine!
Thanks for the patch! |
Fixes #2304
I add the flag
labelrotate=True
, it should fix the problem in most cases.For pie chart with too many sections, I think using leader lines should be a better solution.