Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FIX: make un-used ticks not be visible#10881
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I don't think this is 2.2.3 material, but others may disagree... |
lib/matplotlib/axis.py Outdated
| @@ -1102,7 +1102,12 @@ def _update_ticks(self, renderer): | |||
| tick.set_label1(label) | |||
| tick.set_label2(label) | |||
| if not mtransforms.interval_contains(interval_expanded, loc): | |||
| tick.set_visible(False) | |||
| tick.label1.set_visible(False) | |||
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.
either also mark the gridline and ticks themselves as invisible, or add a comment about what's happening
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.
OK< I can do all that, but its a bit ugly. See the second commit. Tempted to just revert#9452, which I think was handling a bit of an edge-case use.
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.
Or you can just leave a comment on why it is only needed to mark the labels as not visible...
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.
Preference? I don't have one...
I agree its a bit hincky to have the extra ticks. I forget why we do 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.
I have written in many places that we should be stricter and require the ticker to only return visible ticks (after all it has all the info needed to do so). But I guess that's not happening any time soon and instead we just move the issues further down the stack :/
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.
Agenda item for today’s call!
lib/matplotlib/axis.py Outdated
| @@ -1102,7 +1102,12 @@ def _update_ticks(self, renderer): | |||
| tick.set_label1(label) | |||
| tick.set_label2(label) | |||
| if not mtransforms.interval_contains(interval_expanded, loc): | |||
| tick.set_visible(False) | |||
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.
does this line have any effect?
ecda915 to1a14655CompareThere 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.
Might be worth adding the code above as a quick smoke test?
I've put this to |
This is all very confusing, and it seems like a case of whack-a-mole. Isn't there a cleaner solution than putting invisible ticks into a list of ticks to draw? I think @aantzer is pointing to it in his comment#9397 (comment). I suspect the root issue is the interaction between the Locator and autoscaling, and what is needed is an earlier distinction between potential tick locations, relevant to autoscaling, and actual tick locations that will be drawn in the end. This is further complicated by the usual floating point fudge to include a tick that falls an infinitessimal distance beyond the edge. |
Sorry. Thought I’d opened an issue for this. Yes ticks that are returned from the locators should just be the ones thatll be drawn not a couple of extra in case of round off errors. This fix is clearly just a bandaid |
Closing in lieu of#11004, though feel free to resurrect if we want in 2.2.3 |
PR Summary
Addresses#10874
This code fails master:
This was originally caused by#9452 adding string data to the tick labels that were not seen. Previously
get_window_extentwould bail on an empty string. But after#9452 the labels were not empty. But they were never drawn, so they never hadself._rendererassigned. This makes the labels to be visible if they are not visible, and that causesget_window_extentto ignore them.PR Checklist