Set norms using scale names. #20752

anntzer · 2021-07-27T23:28:06Z

PR Summary

Implements #20746.

~~Proof of concept implementation for #20746.~~
~~Still needs a lot of docs, but otherwise mostly works.~~

~~Remaining issues:~~

autoscale() will run into issues for scales with limited domains, e.g. we should/need to be able to auto-infer that LogNorm only normalizes based on the positive values (cf. overrides of autoscale() in LogNorm). Closed by Autoinfer norm bounds. #21989.
The autogenerated norm classes are not picklable, which can be considered a slightly obscure limitation, but can be solved using semi-standard boilerplate as in Templatize class factories. #19033. This is tracked by [Bug]: make_norm_from_scale should create picklable classes even when used in-line. #20755.

PR Checklist

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (run flake8 on changed files to check).
New features are documented, with examples if plot related.
Documentation is sphinx and numpydoc compliant (the docs should build without error).
Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

jklymak · 2021-07-28T15:00:34Z

lib/matplotlib/cm.py

+            colors.Normalize)()
+    norm_cls = type(norm)
+    norm_cls.__name__ = f"{scale_cls.__name__}Norm"
+    return norm_cls


I guess this seems overkill to me, and fragile as shown by the pickling issue? Why don't we just use scale._scale_mapping or make one, i.e. colors._norm_mapping? Do we really need the extra flexibility given by allowing dynamic creation?

I don't really want to create a separate mapping as people can register new scales (e.g. mpl-probscale does it), but they wouldn't show up in the norm mapping if these are separate.

Back when I was a whippersnapper, we only had the linear norm and we liked it....

I'm not sure that making this super-flexible is worth the complication. The strings can be for a few common norms. How many specialized scales truly need a norm as well, and how much of a problem is it to ask users to just pass the norm instead to the string? Its not like probscale is going to be a super-useful color norm. If you wanted to add a register_norm, fine, but making it all dynamic just seems to be asking for trouble.

#20755 fixes that part.

anntzer · 2021-12-21T20:17:36Z

This should now be ready for review. Note that the docs are currently split in a separate commit and most methods now just point to imshow() to reduce the duplication, but once we agree on a wording I can also copy paste the same doc everywhere if that's what we decide to do at the end.

timhoffm

At some later point, we may want to move the explanation of the colormapping mechanism to some Explanations section in the docs and link everything there instead of to imshow(). But linking imshow() for a start is ok.

lib/matplotlib/axes/_axes.py

lib/matplotlib/streamplot.py

greglucas

👍 I like the concept of using strings to make it easier to get a norm than having to import matplotlib.colors and create an object. I think this could lead to some mismatch/confusion about which scale/norm mappings are available, so perhaps some better string validation/error messages would be helpful when using this.

greglucas · 2022-01-21T02:10:14Z

lib/matplotlib/axes/_axes.py

            The `.Normalize` instance used to scale scalar data to the [0, 1]
-            range before mapping to colors using *cmap*. By default, a linear
+            range before mapping to colors using *cmap*.  By default, a linear


There should only be one space after a period for most (all now?) style guides, so this shouldn't be updated. Same for the other docstrings in this PR.

greglucas · 2022-01-21T02:55:31Z

lib/matplotlib/cm.py

        if norm is None:
            norm = colors.Normalize()
+        elif isinstance(norm, str):
+            # case-insensitive, consistently with scale_factory.
+            scale_cls = scale._scale_mapping[norm.lower()]


I'm wondering if it would be helpful to wrap this in a try-except and re-raise with a list of options available for users?

I can see users trying: imshow(..., norm="Boundary") and wondering what option would work instead.

anntzer · 2022-01-21T07:30:27Z

Thanks, I should have addressed all the comments.

jklymak · 2022-01-21T08:26:35Z

lib/matplotlib/cm.py

@@ -331,6 +332,35 @@ def unregister_cmap(name):
    return _cmap_registry.pop(name)


+@functools.lru_cache(None)
+def _auto_norm_from_scale(scale_cls):


Overall I think this is a great idea. However, all the scales with strings currently have Norm classes, don't they? If so, why are you creating a new class here versus using the existing classes? It seems like it will just make things needlessly confusing.

Some scales (most prominently, logit) don't have a norm associated;

because everything is wrapped under lru_caches, _auto_norm_from_scale(LogScale) is going to be exactly the same object as LogNorm (multiple calls to make_norm_from_scale or _auto_norm_from_scale always return the same norm class). (I added a test for that.)

Can you document that here? Because I couldn't parse that from the code, but maybe my ignorance of how the caches get shared across submodules.

However, I'm still not understanding what happens if the user puts 'logit' in here. Are you saying a norm will still be created?

The documentation is the third bullet point below ("The returned norm class is memoized and reused for later calls." But feel free to suggest something clearer). Yes, a norm class will be dynamically created if the user puts in "logit" (and later calls will reuse the same dynamically created norm class).
This will also work with scales provided by third-parties, e.g. mpl-probscale (which is why I did not go for just an explicit and exhaustive mapping of names to norm classes -- this would not work with third-party scales).

this would not work with third-party scale

Fair enough if we are supporting those as norms, though that sounds risky to me...

The returned norm class is memoized and reused for later calls.

Maybe just a bit more verbose "the returned norm class is the same as for make_norm_from_scale (so norm='log' will return a LogNorm instance); these are are also memoized..." (though is there a reason to not use "cached"? "memoized" is pretty jargon, but maybe my ignorance)

Re: 3rd-parties: well, that was also one of the points of the PR (and again, think of this as "supporting putting a third-party scale on a colorbar axis").
Re: docs: sure, reworded, sorry for the jargon :)

Edit: actually I had to fix the caching, but it should be good now.

I had a bit of confusion initially figuring out exactly what is being cached. The cached "thing" is the class definition. It is not the returned Norm object. So, these two calls will have different LogNormalize objects, which is consistent with how it works currently when no norm is given (a new Normalize object is created each time).

im1 = ax.imshow([[0, 1]], norm="log") im2 = ax.imshow([[0, 1]], norm="log") assert im1.norm is not im2.norm

Good point, I further reworded the docs to clarify that.

jklymak

This looks ready to go to me, but we probably need to copy the documentation between the methods. Its not very nice to be asked to look at another doscstring for basic kwargs if we can help it.

jklymak · 2022-02-01T08:48:55Z

lib/matplotlib/axes/_axes.py

+            The normalization can also be given as a str, which should be a
+            scale name (as in `~.Axes.set_xscale`, i.e. one of "linear", "log",
+            "symlog", "logit", etc.). In that case, a normalization class is
+            dynamically generated from the corresponding scale, as if using
+            that scale for the image's colorbar.


Suggested change

The normalization can also be given as a str, which should be a

scale name (as in `~.Axes.set_xscale`, i.e. one of "linear", "log",

"symlog", "logit", etc.). In that case, a normalization class is

dynamically generated from the corresponding scale, as if using

that scale for the image's colorbar.

If a str, *norm* must be a scale name (i.e. "linear", "log",

"symlog", "logit", as per `~.Axes.set_xscale`, or call

`~.scale.get_scale_names`). By default, this scale will be

also used for a colorbar for the image.

I'm not really sure about this wording? (Why "by default"? 1) I guess you could change that, but having a linear-scale colorbar for a lognorm seems rare; 2) more than anything, this kwarg sets the norm on the mappable; setting the colorbar scale is more a side effect of that. Also, even set_xscale doesn't mention get_scale_names. Perhaps it should, but if it doesn't, I don't think imshow() needs to do that either.)

"one of "linear", "log", "symlog", "logit", etc." is vague, so was trying to be more precise. "...as if using the scale" was also theoretical sounding, whereas we do use the scale for the colorbar, so why not be explicit? As for "by default" - the user can change the colorbar scale. But I don't feel strongly about my wording - feel free to iterate as you see fit.

jklymak · 2022-02-01T08:49:47Z

lib/matplotlib/axes/_axes.py

+        cmap, norm, vmin, vmax
+            Data normalization and colormapping parameters for *C*. See
+            `~.Axes.imshow` for a detailed description.


I guess this needs to all be copied over.

anntzer · 2022-06-23T10:13:02Z

Rebased.

timhoffm · 2022-06-23T19:45:57Z

@anntzer It seems that my proposal to interpolate every parameter individually got lost:

marker : ...
    ...

%(param_cmap)s

%(param_norm)s

%(param_vmin_vmax)s

    more text on vmin/vmax.

alpha : float
    ...

IMHO that would solve the major issues with the present interpolation

Naming %(cmap_norm_vmin_vmax_doc)s is quite longish and still not very clear.
There's one place that extends the vmin/vmax section, but it relies implicitly on vmin/vmax being the last parameter in the interpolation block.

What do you think?

anntzer · 2022-06-23T21:37:51Z

Sure, done. (I named them cmap_doc, norm_doc, vmin_vmax_doc as that's more consistent with other _doc entries already present in interpd, but don't feel strongly about that either.)

anntzer · 2022-06-23T21:42:31Z

lib/matplotlib/cm.py

        if norm is None:
            norm = colors.Normalize()
+        elif isinstance(norm, str):
+            try:
+                scale_cls = scale._scale_mapping[norm]


Given #20753, I made this case-sensitive from the beginning.

timhoffm · 2022-06-25T12:57:26Z

Flake8:

./lib/matplotlib/contour.py:1787:1: E124 closing bracket does not match visual indentation
./lib/matplotlib/tri/tricontour.py:210:1: E124 closing bracket does not match visual indentation

The other CI failures can probably be fixed by a rebase.

timhoffm · 2022-06-25T13:03:22Z

lib/matplotlib/cm.py

+    before mapping to colors using *cmap*. By default, a linear scaling mapping
+    the lowest value to 0 and the highest to 1 is used. This parameter is


Suggested change

before mapping to colors using *cmap*. By default, a linear scaling mapping

the lowest value to 0 and the highest to 1 is used. This parameter is

before mapping to colors using *cmap*. By default, a linear scaling is used,

mapping the lowest value to 0 and the highest to 1. This parameter is

IMHO it's a bit easier to read this way.

timhoffm · 2022-06-25T13:08:21Z

lib/matplotlib/cm.py

+    cmap_doc="""\
+cmap : str or `~matplotlib.colors.Colormap`, default: :rc:`image.cmap`
+    The Colormap instance or registered colormap name used to map scalar data
+    to colors. This parameter is ignored for RGB(A) data.""",


I think not all functions using cmap support RGB(A) data. Now that we have entries per parameter it is possible to do move this out of the general description and do

%(cmap_doc)s This parameter is ignored for RGB(A) data.

only when needed (or make a cmap_rgba_doc if there are many cases).

Actually, none of cmap, vmin/vmax, and norm apply for RGBA data, so all of them should get this comment.

Um, that's not what I meant. We have functions like scatter that allow either colormapped or RGBA data. Here, the sentence makes sense. But we also have functions like hexbin, that only operate with colormapping. Mentioning that a parameter is ignored for RGBA data when RGBA data is not supported by the function is somewhat confusing.

Sorry I was unclear, I was meaning "all of cmap, norm, and vmin/vmax should get this comment". From a quick check, the only functions that do not take color input are hexbin and hist2d; do you agree?

From a quick search, I think the following functions support color input:

scatter

hexbin

imshow

pcolor

pcolormesh

pcolorfast

hist2d

specgram

contour

tripcolor

Feel free to update the list.

Even if it's only hexbin and hist2d, how do we handle them?

OK, this should be handled now.

jklymak · 2022-06-30T08:59:14Z

Moving to draft until flake8 looked at....

anntzer · 2022-07-19T20:27:13Z

@jklymak Thanks for the ping, now fixed.

jklymak

Looks good to me!

jklymak · 2022-07-20T00:49:47Z

Did we want the three commits?

anntzer · 2022-07-20T06:50:43Z

This was mostly to make it easier to iterate on the individual parts separately, now squashed.

I don't know what's the status of 3.6 right now; @QuLogic can I remilestone this to 3.6?

jklymak · 2022-07-20T14:43:57Z

I don't think there is a 3.6.x branch, and I've not heard that the release has been started, so lets merge as 3.6

anntzer added topic: color/color & colormaps topic: transforms and scales labels Jul 27, 2021

anntzer mentioned this pull request Jul 27, 2021

Deprecate support for case-insensitive scales. #20753

Merged

7 tasks

anntzer force-pushed the strnorms branch from 17aeeea to fa9e27d Compare July 28, 2021 08:05

This was referenced Jul 28, 2021

[Bug]: make_norm_from_scale should create picklable classes even when used in-line. #20755

Closed

autoinfer norms from scale names in the "norm" kwarg of imshow() and friends. #20746

Closed

jklymak reviewed Jul 28, 2021

View reviewed changes

anntzer force-pushed the strnorms branch from fa9e27d to 3a4fe48 Compare December 10, 2021 12:12

anntzer added the status: waiting for other PR label Dec 10, 2021

anntzer mentioned this pull request Dec 16, 2021

Autoinfer norm bounds. #21989

Merged

6 tasks

anntzer force-pushed the strnorms branch from 3a4fe48 to ae9c7a8 Compare December 21, 2021 20:16

anntzer marked this pull request as ready for review December 21, 2021 20:16

anntzer removed the status: waiting for other PR label Dec 21, 2021

anntzer added this to the v3.6.0 milestone Dec 21, 2021

anntzer force-pushed the strnorms branch 2 times, most recently from 9715b10 to ae7c8e8 Compare December 21, 2021 20:27

jklymak requested review from timhoffm and jklymak January 20, 2022 20:42

timhoffm reviewed Jan 20, 2022

View reviewed changes

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

lib/matplotlib/streamplot.py Outdated Show resolved Hide resolved

greglucas reviewed Jan 21, 2022

View reviewed changes

anntzer force-pushed the strnorms branch from ae7c8e8 to 84c09d5 Compare January 21, 2022 07:30

jklymak reviewed Jan 21, 2022

View reviewed changes

anntzer force-pushed the strnorms branch 5 times, most recently from e0de415 to d38dd06 Compare January 21, 2022 14:44

jklymak reviewed Feb 1, 2022

View reviewed changes

anntzer force-pushed the strnorms branch from 319a3ea to 97fafd1 Compare June 23, 2022 10:10

anntzer marked this pull request as ready for review June 23, 2022 10:13

anntzer removed the status: needs rebase label Jun 23, 2022

anntzer force-pushed the strnorms branch from 97fafd1 to 61179b2 Compare June 23, 2022 21:37

anntzer force-pushed the strnorms branch from 61179b2 to 649eb06 Compare June 23, 2022 21:41

anntzer commented Jun 23, 2022

View reviewed changes

timhoffm reviewed Jun 25, 2022

View reviewed changes

jklymak marked this pull request as draft June 30, 2022 08:59

anntzer force-pushed the strnorms branch from 649eb06 to c6772c0 Compare July 3, 2022 17:58

QuLogic modified the milestones: v3.6.0, v3.7.0 Jul 5, 2022

anntzer marked this pull request as ready for review July 14, 2022 10:14

anntzer force-pushed the strnorms branch from c6772c0 to 09273fd Compare July 14, 2022 10:14

jklymak approved these changes Jul 20, 2022

View reviewed changes

Set norms using scale names.

7da2f71

anntzer force-pushed the strnorms branch from 09273fd to 7da2f71 Compare July 20, 2022 06:50

jklymak modified the milestones: v3.7.0, v3.6.0 Jul 20, 2022

jklymak merged commit 3992fb0 into matplotlib:main Jul 20, 2022

anntzer deleted the strnorms branch July 20, 2022 15:19

anntzer mentioned this pull request Apr 19, 2024

[Bug]: Setting norm by string doesn't work for hexbin #28105

Closed

		before mapping to colors using cmap. By default, a linear scaling mapping
		the lowest value to 0 and the highest to 1 is used. This parameter is

Set norms using scale names. #20752

Set norms using scale names. #20752

Conversation

anntzer commented Jul 27, 2021 • edited Loading

PR Summary

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Dec 21, 2021

timhoffm left a comment

Choose a reason for hiding this comment

greglucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Jan 21, 2022

Choose a reason for hiding this comment

anntzer Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklymak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Jun 23, 2022

timhoffm commented Jun 23, 2022 • edited Loading

anntzer commented Jun 23, 2022

Choose a reason for hiding this comment

timhoffm commented Jun 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer Jun 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklymak commented Jun 30, 2022

anntzer commented Jul 19, 2022

jklymak left a comment

Choose a reason for hiding this comment

jklymak commented Jul 20, 2022

anntzer commented Jul 20, 2022 • edited Loading

jklymak commented Jul 20, 2022

anntzer commented Jul 27, 2021 •

edited

Loading

anntzer Jan 21, 2022 •

edited

Loading

timhoffm commented Jun 23, 2022 •

edited

Loading

timhoffm commented Jun 25, 2022 •

edited

Loading

anntzer Jun 25, 2022 •

edited

Loading

anntzer commented Jul 20, 2022 •

edited

Loading