-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix colorbar bad ticks #11556
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
Fix colorbar bad ticks #11556
Conversation
I mentioned in a few places that it should really be the responsibility of the ticker to return ticks that are indeed between vmin and vmax (should perhaps start a metaissue on that :)); looks like this is yet another reason to require so. Won't block this quickfix of course but perhaps leave a comment that it doesn't address the root issue (so that we can remove this patch once the real fix above is implemented). |
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.
Good, thank you.
Well, the docstring of the classes explicitly state that they work around this limitation - the problem here was that I forgot to actually do it for |
1ef04eb
to
dc2fce2
Compare
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.
Please squash.
Anyone can (including @jklymak ) can dismiss thing and merge once squashed and CI passes.
dc2fce2
to
d5dae3c
Compare
@meeseeksdev backport to v2.2.x |
There seem to be a conflict, please backport manually |
probably not worth the backport wrigamarole... |
PR Summary
As noted in issue #11510, extra ticks can get into the colorbar that extend past the minimum or maximum due to the presence of the "extend='both/lower/upper'` kwarg. This fixes that issue for major ticks.
An enhancement would be to come up with a
cb.minorticks_on
function to colorbar that supplies the appropriately trimmed minor tick locator as well. That can be a further PR. But this really fixes a bug in the existing functionality...Before:
After
PR Checklist