-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Let Formatters format all ticks at once. #12909
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
b7adb3e
to
c867b93
Compare
I like this and agree it makes a lot more sense, but... If someone calls |
For |
So, can you implement |
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.
modulo an api_changes or whats_new entry.
As per above - needs change log and ideally a real-world example... |
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.
Apart from the one suggested API change, and the need for an API note, I like this.
The real-world example is basically ConciseDateFormatter (happy to rewrite it using this API after the ConciseDateFormatter is merged). |
Happy to do it the other way around if that makes more sense. |
I think it makes more sense to get ConciseDate in first and then we'll be better able to see whether this PR fixes the design properly, or if we need some other API. |
ping @anntzer ConciseDateFormatter is now in, so this can be looked at again? I'm not sure how this plays with the issues of the minor locator ignoring major ticks. They are separate issues, right? |
Pushed an adaptation of ConciseDateFormatter using the format_ticks API, it is a bit simpler. |
LGTM! Still needs the API change note? |
This is useful especially when labels are interdependent (e.g. ConciseDateFormatter, but also LogFormatter which currently relies on set_locs, etc.). Because `set_major_formatter`/`set_minor_formatter` (somewhat unusually) enforces that its argument is a Formatter subclass, even third-party formatters will inherit from the base class and have this method defined.
added whatsnew |
I think I made a mistake while reimplementing ConciseDateFormatter; namely, the "compression" should be used regardless of whether show_offset is True or False and show_offset only controls whether offset_string is set. Force-pushed a fix restoring the behavior as intended by @jklymak. |
ooops, thanks for catching that.... |
Travis failure is the unrelated, spurious wqy-zenhei.ttc test. |
Apologies if not the right place. This PR has the following side effect: fig, ax = plt.subplots()
data = np.clip(randn(250, 250)*1000, 0, 1000)
cax = ax.imshow(data, norm=mpl.colors.LogNorm(vmin=1, vmax=1000))
cbar = fig.colorbar(cax, format='%d')
cbar.minorticks_off()
cbar.set_ticks([5, 10, 100])
plt.show() Prior to this PR, minor ticks would remain off after Edit: Comment is wrong, this PR did not change this. |
This is useful especially when labels are interdependent (e.g.
ConciseDateFormatter, but also LogFormatter which currently relies on
set_locs, etc.). xref #10841, maybe of interest for @jklymak.
Because
set_major_formatter
/set_minor_formatter
(somewhat unusually)enforces that its argument is a Formatter subclass, even third-party
formatters will inherit from the base class and have this method
defined.
PR Summary
PR Checklist