-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Polar tick improvements #9068
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
Polar tick improvements #9068
Conversation
Would like to get this in to 2.1, since it's a good follow-on to #4699. Just need to figure out why it's not passing somehow. |
df70809
to
c753077
Compare
Ah, all set now; I wonder if that |
The r-grid ticks do not look like they are pulling the tangent from the right place? Over all 👍 |
I think the new label directions are much more un-readable than having them all horizontal. It also doesn't look like there are any image tests for angular ticks on the outside of the axes? |
I'm +0.5 on having the capability to turn on the rotated labels, -1000 on having it be the default. This belonged in 2.0 if we were going to do it since this is a stylistic change--I really don't think our users will view this as a clear enhancement. (My first reaction honestly was "Ewww.") |
Just a quick thought... should we make sure the api for this matches the
api for rotated labels in pie()?
…On Tue, Aug 29, 2017 at 12:28 PM, Ryan May ***@***.***> wrote:
I'm +0.5 on having the capability to turn on the rotated labels, -1000 on
having it be the default. This belonged in 2.0 if we were going to do it
since this is a stylistic change--I really don't think our users will view
this as a clear enhancement. (My first reaction honestly was "Ewww.")
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9068 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-BWE6RB7Z9LtfUqmmq-ndz8Rm3deks5sdDw-gaJpZM4O933_>
.
|
This could be a useful option--but only if the upside-down labels are flipped 180 degrees to bring them right-side-up. In other words, label orientations should be kept within +-90 degrees, not +-180. |
Just took a look at the rotatelabels feature of pie(), and it made me
realize that there might be two ways people would want the labels oriented:
perpendicular to the normal, or parallel to the normal. Right now, pie()'s
(non-default) rotatelabels feature will orient the labels parallel to the
normal, and also will flip the text such that it is never upside down.
…On Tue, Aug 29, 2017 at 12:56 PM, Eric Firing ***@***.***> wrote:
This could be a useful option--but only if the upside-down labels are
flipped 180 degrees to bring them right-side-up. In other words, label
orientations should be kept within +-90 degrees, not +-180.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#9068 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-ATabNBQFTqrXcVcmz5qCppbcu0eks5sdEKNgaJpZM4O933_>
.
|
So an informal survey by posting an image (from the test suite changes) to Twitter would seem to argue that this change is by no means a slam dunk in terms of improving default plot quality. I do think there's a strong case to be made for having the option to turn this behavior on, though. |
FWIW, even if one decides to keep the current default behavior, I would advocate for keeping the 2 new polar-tick classes and an ad-hoc example somewhere. IMHO, the rotated radial ticks could sometimes be more readable than the current ones, especially when cluttering is not far away, for example (from the test images): For the theta-tick label, I think that having an option to limit the rotation domain as @efiring suggested, as well as @WeatherGod´s one about allowing the labels to be parallel to the normal could be nice additions. If I may, I would also suggest to have an option to rotate the theta-tick labels outwards, i.e. + 180° compared to the current rotated ones. Possible uses of rotated theta-tick labels that come to my mind are sky/constellation charts (random examples here and here) |
To summarize, I'd like this on for wedges, but there already many special cases to preserve the old behaviour with full circle plots, so I'm fine with adding a few more to do the same for tick labels. I don't think there's any point in preserving old ticks themselves because they were useless before. |
To be clear I think the new code with proper polar ticks is a nice upgrade. I just think the new alignment/rotation constitutes a style change that would not be universally well-received, so maintaining the old look (at least for full-circle plots) is the way to go. |
Also, I'm 👎 on holding up cutting the RC for this, so somebody had better get working on the modifications to make this behavior optional if it's going to land in the RC this week. 😈 |
2c from the peanut gallery: the rotated labels would probably look better if their "horizontal" (= in the axis of the text) center was tangent to the circle, not their "left" edge. |
It's straightforward to replicate old behaviour because there are already several checks for full-circle vs wedges. The only problem is that now that we are properly interpreting tick padding, the default is too small on figures of the size in the tests. Note though that since previously the angles were just offset by a fixed radial distance, any other sized figure might have had too much or too little padding with no way to fix it. I'm not really sure what to do about this. Maybe just add extra padding automatically? |
I'm not totally sure I like that hack to get padding to match previous tests. Maybe the best option is to add an rcParam for polar plots with a better default (but this is a lot more work)? |
On a bit more consideration, I am convinced that changing the default rotation is a style change which we have already determined is an API break so 👍 to these classes going in 👎 to rotating the tick labels by default on full circles 👍 to rotated by default on wedges (as those are new) and 👍 to a convenience method on the axes objects to apply the rotation. I am 50/50 on letting this be merged post RC assuming it can be made to not break tests. |
Current results are 99% the same; it's very hard to get the padding exactly the same since it was never quite correct in the first place. I keep forgetting that there are a couple polar tests that output multiple files, so that's why tests are failing right now. |
I am fine with minor padding shifts. |
https://github.com/matplotlib/matplotlib/pull/9068/files#diff-f1d864385683ce9e37a13df3975627c9 the labels have been squished up into the ticks. |
Sure, give me a couple hours to make sure everything works and I can take care of it. |
This is able to add a position-dependent rotation to the tick marker, ensuring it is correctly perpendicular to the spine. It also rotates the tick label so that it is parallel to the spine.
Just like ThetaTick, these allow for text and tick to be correctly perpendicular to the spine.
This enables checking that they appear correctly rotated.
3547653
to
d8fa323
Compare
This option enables the automatic rotation of polar tick labels.
d8fa323
to
549c07f
Compare
I had rebased this earlier today and added the 'auto' option. Just now I made a small tweak so that 'auto' will also apply to full circles (since presumably that's what the user wants if they've explicitly asked for it) and modified one test to check that that works. The last two commits (before the image change) are the new things. |
doc/users/whats_new.rst
Outdated
@@ -62,6 +62,13 @@ negative values are simply used as labels, and the real radius is shifted by | |||
the configured minimum. This release also allows negative radii to be used for | |||
grids and ticks, which were previously silently ignored. | |||
|
|||
Radial ticks have be modified to be parallel to the circular grid line. Angular |
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.
Oops, small grammar typo here.
ENH: Polar tick improvements
backported to v2.1.x as 6889c6d |
if rotation of the theta labels is used in conjunction with ax.set_theta_zero_location( "N") and or ax.set_theta_direction( 1 ), the labels seem to rotate weirdly as you go around the theta axis. The following code should illustrate what I mean, the label flips through 180 degrees as you move from 90 to 135. I'm on a windows 10 PC & using python 2.7 btw. import matplotlib.pyplot as plt fig = plt.figure() plt.show() |
That is fixed by #9881. |
PR Summary
Semi-WIP as some bits of this are a bit hacky, so they might need to be corrected a bit. This improves upon ticks used in polar plots by correctly rotating them to be perpendicular to the spine. Tick labels also now use the configured padding instead of some fixed value radially; this is a big difference if the radial limits are small or large.
PR Checklist