-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Make tick_left/right keep labels off if they are already off #9670
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
Does this address the corresponding changes in |
@mwaskom Not yet - see todo list above... Note its also not working yet |
details, details... |
This definitely needs a test! |
@dstansby the test is easy if this is the behaviour we want. |
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 reasonable as a fix for the regression. I suspect there is no perfect fix. The underlying problem is that our API is complicated and confusing, with too many ways to do some things, and a lack of clarity such as is illustrated by the original docstring 'use ticks only on top'.
@efiring Yeah, I would actually be in favour of deprecating these functions due to their ambiguity. This PR at least tries to document the ambiguity, but its suboptimal. OTOH, the |
Deprecation sounds good to me! It's hard to imagine that these vague shorthand versions are used so often in interactive mode, where their brevity would be most advantageous. |
Backport PR #9670 on branch v2.1.x
PR Summary
This modifies
XAxis.tick_left
andX-axis.tick_right
to keep the tick labels turned off if the majorlabels are already turned off.
Not 100% convinced this is the right thing to do, but fixes #9664 Wild guess is that this doesn't break anything, whereas the new behaviour did.
Fixes:
Breaks:
The following if the user expected
tick_left()
to turn the labeling back on.I'm not sure why this would happen, but this PR breaks that case...
PR Checklist