ENH: adjustable colorbar ticks#9903
Conversation
|
OK, this failed enough tests that there are a bunch more edge cases I need to consider. Good thing there are tests. I'll probably re-do this from scratch and try to make it less invasive. I do think this is reasonable for 2.2, though, because I'll keep plugging away at it. |
9d1146d to
5c48c10
Compare
|
This is redone, and much simpler now. 4 image files had to be changed... |
5c48c10 to
1d4160c
Compare
338c0a1 to
a31cadd
Compare
|
Ping @efiring The diff looks worse than it is because I moved a bit of code around. |
|
|
||
| self._set_label() | ||
|
|
||
| def _get_ticker_locator_formatter(self): |
There was a problem hiding this comment.
Much of this was jut factored out of _ticker() But note the new locators for LogNorm and the fallback...
dstansby
left a comment
There was a problem hiding this comment.
Some questions/comments, looks like this is good overall though
lib/matplotlib/colorbar.py
Outdated
| This locator is just a `.MaxNLocator` except the min and max are | ||
| clipped by the norm's min and max (i.e. vmin/vmax from the | ||
| image/pcolor/contour object). This is necessary so ticks don't | ||
| extrude into the "extend regions". |
There was a problem hiding this comment.
Shouldn't the docstring line up with the left of the quote marks?
lib/matplotlib/colorbar.py
Outdated
| extrude into the "extend regions". | ||
| """ | ||
|
|
||
| def __init__(self, colorbar, *args, **kwargs): |
There was a problem hiding this comment.
Why are there args and kwargs here if they're not used? Can this also get a docstring (just for the colorbar parameter is probably fine)
| extrude into the "extend regions". | ||
|
|
||
| """ | ||
| def __init__(self, colorbar, *args, **kwargs): |
There was a problem hiding this comment.
Same as above comment on __init__
lib/matplotlib/colorbar.py
Outdated
| self._config_axes(X, Y) | ||
| if self.filled: | ||
| self._add_solids(X, Y, C) | ||
| # self._set_view_limits() |
There was a problem hiding this comment.
Is this old debug code that needs to be removed?
lib/matplotlib/colorbar.py
Outdated
| if (isinstance(self.norm, colors.LogNorm) | ||
| and self._use_adjustable()): | ||
| ax.set_xscale('log') | ||
| ax.set_yscale('log') |
There was a problem hiding this comment.
and only the yscale be log here?
There was a problem hiding this comment.
They both need to be log to make the aspect kwarg work properly (... or we could do a bunch of math and make the X values make the aspect ratio right.)
|
|
||
| self._set_label() | ||
|
|
||
| def _get_ticker_locator_formatter(self): |
There was a problem hiding this comment.
This method could do with a quick comment underneath to explain what it does.
lib/matplotlib/colorbar.py
Outdated
| else: | ||
| b = self._boundaries[self._inside] | ||
| locator = ticker.FixedLocator(b, nbins=10) | ||
| # locator, formatter = _get_ticker_locator_formatter() |
There was a problem hiding this comment.
This commented out line can go
452e401 to
5972691
Compare
|
Pushed to 3.0 as this isn't urgent. But I think it'd be easy to put in 2.2 if someone wants to dive in and re-milestone... |
5972691 to
402b4d3
Compare
|
rebased |
|
The before-and-after comparison looks great, but tests seem to be failing now. |
|
Yeah, its just the streamplot image test at 0.036 tolerance. I'll try to track it down. It passes on my machine and I took the text out so it can't be freetype. Grrrr. |
|
I'm not sure that this is the right place to tell this, but I recently wrote a detailed instruction on colorbar in matplotlib which might contribute for improving tutorials about colorbar. After writing it, fortunately or not, I found this PR which seems to change colorbar's behavior significantly (no pseudo minor ticks, no normalization of vmin/vmax to 0/1, for example). Actually these modified behaviors are parts of the main topics in my instruction. Then I'm wondering whether I should translate the introduction as it is into English and make a PR to improve docs for the current behavior of colorbar (about cbar.ticker, especially), or I should wait for this PR to be implemented in, say, next rc version and revise the instruction to catch up the new behaviors. What do you think? |
|
@skotaro Sounds like we had some similar difficulties with colorbars. I will push in the near future for this PR to get in, but it won’t be part of the distribution until 3.0 (July/Aug). You may want to checkout constrained layout in 2.2Rc1 for easier colorbar placement. That doesn’t preclude a tutorial that explains how to fix things manually, but I’d suggest such a tutorial should at least reference the automated methods. |
|
The problem with the doc build is that after the merge, the sphinx conf.py |
651d609 to
2c33640
Compare
|
Somehow the doc build didn't like the name I had for the whats-new file. Works fine now. 🤷♂️ |
lib/matplotlib/colorbar.py
Outdated
| ax.xaxis.get_major_formatter().set_offset_string(offset_string) | ||
| _log.debug('Using fixed locator on colorbar') | ||
| ticks, ticklabels, offset_string = self._ticker(locator, formatter) | ||
| if self.orientation == 'vertical': |
There was a problem hiding this comment.
You can use the long_axis, short_axis here, too. Initialize them prior to the _use_auto_colorbar_locator check.
Instead of ax.set_yticklabels you can use the equivalent short_axis.set_ticklabels, etc.
2c33640 to
252ef97
Compare
6bdc427 to
52e5d81
Compare
|
|
||
| Colorbar ticks now adjust for the size of the colorbar if the | ||
| colorbar is made from a mappable that is not a contour or | ||
| doesn't have a BoundaryNorm, or boundaries are not specified. |
lib/matplotlib/colorbar.py
Outdated
|
|
||
| def __init__(self, colorbar): | ||
| """ | ||
| _ColorbarAutoLocator(colorbar) |
There was a problem hiding this comment.
Is there a particular need to include the signature?
lib/matplotlib/colorbar.py
Outdated
| if type(self.norm) == colors.LogNorm: | ||
| long_axis.set_minor_locator(_ColorbarLogLocator(self, | ||
| base=10., subs='auto')) | ||
| long_axis.set_minor_formatter(ticker.NullFormatter()) |
There was a problem hiding this comment.
Need to check that some ticks are labelled...
There was a problem hiding this comment.
Fixed. LogLocator works fine here.
|
@jklymak Is there an obvious reason why the ticks of the colorbar in the bottom right panel are not symmetrical (in log space) around 1 (e.g. [1e3, 1e0, 1e-3], instead of [1e2, 1e-1]), while the old one were “quite” symmetrical? |
|
@afvincent because the LogLocator only returned two tick marks as being able to fit and it chose those? Its just using the default LogLocator - i.e. if you had a normal yaxis that size, those are the ticks you'd get. |
|
@skotaro Are you still interested in including your (extensive!) colorbar tutorial into the mpl docs? Even if it needs to be updated following this PR and/or to take constrained_layout into account, feel free to open an early issue/PR to track the work. |
PR Summary
This PR changes colorbar to draw from
self.vmintoself.vmaxinstead of from 0 to 1 so thatAutoTickLocatorcan work. Only modifies ticks if no boundaries are specified (i.e. contours).See #9246
example:
before:
after:
PR Checklist