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
ENH: Add func norm #18653
Conversation
989c966
to
15af903
Compare
15af903
to
54be491
Compare
54be491
to
b1c5a0c
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.
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! |
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
pytest
passes).flake8
on changed files to check).flake8-docstrings
andpydocstyle<4
and 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).