Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
ENH: Add func norm#18653
Uh oh!
There was an error while loading.Please reload this page.
Conversation
989c966 to15af903CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| 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 becauseinverse doesn't passvalue throughprocess_value like__call__ does. Something like:
diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.pyindex 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 thenp.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 thenp.array from the inverse check?
Uh oh!
There was an error while loading.Please reload this page.
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).