-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
ENH: Add func norm #18653
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
ENH: Add func norm #18653
Conversation
989c966 to
15af903
Compare
15af903 to
54be491
Compare
54be491 to
b1c5a0c
Compare
timhoffm
left a comment
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.
Minor style comments.
7ab90e3 to
1c043b7
Compare
| return 10**x | ||
| norm = mcolors.FuncNorm((forward, inverse), vmin=0.1, vmax=10) | ||
| lognorm = mcolors.LogNorm(vmin=0.1, vmax=10) | ||
| assert_array_almost_equal(norm([0.2, 5, 10]), lognorm([0.2, 5, 10])) |
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.
Did you not want to assert the inverse here as well?
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 wasn't going to but it does drop our test coverage, so added...
Interestingly the _make_norm_from_scale inverse doesn't work on a list of floats (i.e. [1, 2, 3]), but only on a numpy array (np.array([1, 2, 3])) whereas the forward works on a bare list. Not sure if that is a bug. @anntzer ?
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.
Probably because inverse doesn't pass value through process_value like __call__ does. Something like:
diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.py
index e417b8178d..b37ec947fa 100644
--- a/lib/matplotlib/colors.py
+++ b/lib/matplotlib/colors.py
@@ -1449,12 +1449,14 @@ def _make_norm_from_scale(scale_cls, base_norm_cls=None, *, init=None):
t_vmin, t_vmax = self._trf.transform([self.vmin, self.vmax])
if not np.isfinite([t_vmin, t_vmax]).all():
raise ValueError("Invalid vmin or vmax")
+ value, is_scalar = self.process_value(value)
rescaled = value * (t_vmax - t_vmin)
rescaled += t_vmin
- return (self._trf
- .inverted()
- .transform(rescaled)
- .reshape(np.shape(value)))
+ t_value = (self._trf
+ .inverted()
+ .transform(rescaled)
+ .reshape(np.shape(value)))
+ return t_value[0] if is_scalar else t_value
Norm.__name__ = base_norm_cls.__name__
Norm.__qualname__ = base_norm_cls.__qualname__Maybe it also needs the masking?
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.
Looks right. But maybe orthogonal to this PR. I opened an issue in #19239 just to keep them separate...
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.
Since that is now fixed, do you want to remove the np.array from the inverse check?
|
@timhoffm I think I addressed your concern above? Thanks! |
QuLogic
left a comment
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.
Feel free to merge after.
| return 10**x | ||
| norm = mcolors.FuncNorm((forward, inverse), vmin=0.1, vmax=10) | ||
| lognorm = mcolors.LogNorm(vmin=0.1, vmax=10) | ||
| assert_array_almost_equal(norm([0.2, 5, 10]), lognorm([0.2, 5, 10])) |
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.
Since that is now fixed, do you want to remove the np.array from the inverse check?
1f91fcf to
a3bfa93
Compare
PR Summary
Derives a func norm from func scale. From the example:
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsandpydocstyle<4and 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).