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: fix colorbars with no scales#20327
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
2177d8c to52a6aa8Compare| @@ -1152,7 +1152,7 @@ def __init__(self, vmin=None, vmax=None, clip=False): | |||
| self.vmin = _sanitize_extrema(vmin) | |||
| self.vmax = _sanitize_extrema(vmax) | |||
| self.clip = clip | |||
| self._scale =scale.LinearScale(axis=None) | |||
| self._scale = None # will default to LinearScale for colorbar | |||
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.
Note downstream libraries will likely derive from Normalize, so scale should be None so their norm gets used...
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.
@dstansby can you see if this version works?
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.
Yep, seems to work well with this version.
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.
Any chance of a formal review?
7867fc3 to1e8658dCompareUh oh!
There was an error while loading.Please reload this page.
| @image_comparison(['colorbar_twoslope.png'], remove_text=True, | ||
| style='mpl20') | ||
| def test_twoslope_colorbar(): | ||
| # Note that the first tick = 20, and should be in the middle |
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.
Adding the ticklabels to the image would be helpful here.
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.
Yeah, I agree, but we really try to avoid labels, because they cause image comparison problems when the font library changes...
Uh oh!
There was an error while loading.Please reload this page.
87b8ae7 tod1c5a6aCompare
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Closes#20324
The colorbar re-write#20054, needs a scale for the colorbar. Which worked great for most of our Norms because they are all created from a scale now:
matplotlib/lib/matplotlib/colors.py
Line 1435 in08f4629
However, astropy did not derive their norms from a scale, so they had a failure (which I hope this fixes).astropy/astropy#11800
We also had norms like
TwoSlopeNormthat do not derive from a scale, though they probably should/could.Here the proposal is that if there is no scale on the norm, the scale is a
functionscale with the forward and inverse from the norm. This works fine forTwoSlopeNorm(ahem, after we define the inverse for it, which really should have been done when it went in; that took a bit of work to realize).PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).