Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Changing get_cmap to return copies of the registered colormaps.#16943
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.
Changes fromall commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -98,9 +98,9 @@ deprecation warning. | ||
| `~.Axes.errorbar` now color cycles when only errorbar color is set | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| Previously setting the *ecolor* would turn off automatic color cycling for the plot, leading to the | ||
| the lines and markers defaulting to whatever the first color in the color cycle was in the case of | ||
| multiple plot calls. | ||
| `.rcsetup.validate_color_for_prop_cycle` now always raises TypeError for bytes input | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| @@ -147,3 +147,11 @@ The parameter ``s`` to `.Axes.annotate` and `.pyplot.annotate` is renamed to | ||
| The old parameter name remains supported, but | ||
| support for it will be dropped in a future Matplotlib release. | ||
| ``get_cmap()`` now returns a copy of the colormap | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| Previously, calling ``get_cmap()`` would return | ||
| the built-in Colormap. If you made modifications to that colormap, the | ||
| changes would be propagated in the global state. This function now | ||
| returns a copy of all registered colormaps to keep the built-in | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. "... copy of the requested colormap..."? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Should this even go into this now if it is a future change and there are not currently any behavior changes? My thoughts are to actually break these commits up into two separate PRs. One with the deprecation warning/preparation for a future version. Then another one with the proposed copy code in it that could be pushed off for quite a while and discussed for immutability or not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I personally think the globals should be immutable. If folks are modifying the global to then call again as a named colormap, then I think they were using a misfeature/bug. I just gave a comment here because I was reading it, and had a quick thought. Please work out w/ Tim Hoffman how to do the rest of the PR, but this seems to be the right direction to be heading. | ||
| colormaps untouched. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -15,6 +15,7 @@ | ||
| normalization. | ||
| """ | ||
| import copy | ||
| import functools | ||
| import numpy as np | ||
| @@ -70,9 +71,9 @@ def _gen_cmap_d(): | ||
| cmap_d = _gen_cmap_d() | ||
| # locals().update(copy.deepcopy(cmap_d)) | ||
| locals().update(cmap_d) | ||
| # Continue with definitions ... | ||
| @@ -95,6 +96,9 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): | ||
| and the resulting colormap is registered. Instead of this implicit | ||
| colormap creation, create a `.LinearSegmentedColormap` and use the first | ||
| case: ``register_cmap(cmap=LinearSegmentedColormap(name, data, lut))``. | ||
| If *name* is the same as a built-in colormap this will replace the | ||
| built-in Colormap of the same name. | ||
| """ | ||
| cbook._check_isinstance((str, None), name=name) | ||
| if name is None: | ||
| @@ -105,6 +109,8 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): | ||
| "Colormap") from err | ||
| if isinstance(cmap, colors.Colormap): | ||
| cmap_d[name] = cmap | ||
| # We are overriding the global state, so reinitialize this. | ||
| cmap._global_changed = False | ||
| return | ||
| if lut is not None or data is not None: | ||
| cbook.warn_deprecated( | ||
| @@ -118,21 +124,24 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): | ||
| lut = mpl.rcParams['image.lut'] | ||
| cmap = colors.LinearSegmentedColormap(name, data, lut) | ||
| cmap_d[name] = cmap | ||
| cmap._global_changed = False | ||
| def get_cmap(name=None, lut=None): | ||
| """ | ||
| Get a colormap instance, defaulting to rc values if *name* is None. | ||
| Colormaps added with :func:`register_cmap`with the same name as | ||
| built-in colormaps will replace them. | ||
| Parameters | ||
| ---------- | ||
| name : `matplotlib.colors.Colormap` or str or None, default: None | ||
| If a `.Colormap` instance, it will be returned. | ||
| Otherwise, the name of a colormap known to Matplotlib, which will | ||
| be resampled by *lut*. Currently, this returns the global colormap | ||
| instance which is deprecated. In Matplotlib 4, a copy of the requested | ||
| Colormap will be returned. The default, None, means :rc:`image.cmap`. | ||
| lut : int or None, default: None | ||
| If *name* is not already a Colormap instance and *lut* is not None, the | ||
| colormap will be resampled to have *lut* entries in the lookup table. | ||
| @@ -143,7 +152,17 @@ def get_cmap(name=None, lut=None): | ||
| return name | ||
| cbook._check_in_list(sorted(cmap_d), name=name) | ||
| if lut is None: | ||
| if cmap_d[name]._global_changed: | ||
| cbook.warn_deprecated( | ||
| "3.3", | ||
| message="The colormap requested has had the global state " | ||
| "changed without being registered. Accessing a " | ||
| "colormap in this way has been deprecated. " | ||
| "Please register the colormap using " | ||
| "plt.register_cmap() before requesting " | ||
| "the modified colormap.") | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I understand this, but only because I read the thread. If I was a user who ran plt.get_cmap('viridis').set_bad('k')fig,ax=plt.subplots()ax.imshow(data,cmap='viridis') I'd have no idea what I was supposed to do here. I also don't think we should be encouraging any changing of the global state. I'd far rather tell folks that they should deal with the cmap object which is far less arcane than registering the global state: cmp=plt.get_cmap('viridis').set_bad('k')fig,ax=plt.subplots()ax.imshow(data,cmap=cmp) To that end, I'd say "Modifying a registered colormap is deprecated; make a new copy via cmp=plt.get_cmap('viridian') and modify the copy." ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Unfortunately, that won't even work either! (which is why I think this is a bug) You would need to cmp=copy.copy(plt.get_cmap('viridian')) currently to get the desired functionality. The real problem is that further down if you go and request 'viridis' from a plot after doing a bunch of stuff, you will still get the bad colormap. | ||
| return cmap_d[name] | ||
| # return copy.copy(cmap_d[name]) | ||
| else: | ||
| return cmap_d[name]._resample(lut) | ||