From fb902f735995372f345a8333804f5c6052f29770 Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Fri, 4 Mar 2022 07:59:08 -0700 Subject: [PATCH 1/2] MNT: Remove cmap_d colormap access --- .../deprecations/22298-GL.rst | 5 + .../backends/qt_editor/figureoptions.py | 4 +- lib/matplotlib/cm.py | 161 ++++++++---------- lib/matplotlib/colors.py | 21 +-- lib/matplotlib/tests/test_colors.py | 48 +----- lib/matplotlib/tests/test_pickle.py | 2 +- 6 files changed, 82 insertions(+), 159 deletions(-) create mode 100644 doc/api/next_api_changes/deprecations/22298-GL.rst diff --git a/doc/api/next_api_changes/deprecations/22298-GL.rst b/doc/api/next_api_changes/deprecations/22298-GL.rst new file mode 100644 index 000000000000..814f7c4e48bc --- /dev/null +++ b/doc/api/next_api_changes/deprecations/22298-GL.rst @@ -0,0 +1,5 @@ +``cmap_d`` removal +~~~~~~~~~~~~~~~~~~ + +The deprecated ``matplotlib.cm.cmap_d`` access to global colormaps +has been removed. diff --git a/lib/matplotlib/backends/qt_editor/figureoptions.py b/lib/matplotlib/backends/qt_editor/figureoptions.py index 31e929c4a7c2..b7c42028e00e 100644 --- a/lib/matplotlib/backends/qt_editor/figureoptions.py +++ b/lib/matplotlib/backends/qt_editor/figureoptions.py @@ -135,10 +135,10 @@ def prepare_data(d, init): continue labeled_mappables.append((label, mappable)) mappables = [] - cmaps = [(cmap, name) for name, cmap in sorted(cm._cmap_registry.items())] + cmaps = [(cmap, name) for name, cmap in sorted(cm._colormaps.items())] for label, mappable in labeled_mappables: cmap = mappable.get_cmap() - if cmap not in cm._cmap_registry.values(): + if cmap not in cm._colormaps.values(): cmaps = [(cmap, cmap.name), *cmaps] low, high = mappable.get_clim() mappabledata = [ diff --git a/lib/matplotlib/cm.py b/lib/matplotlib/cm.py index a2b0a75816d4..0d3aec035858 100644 --- a/lib/matplotlib/cm.py +++ b/lib/matplotlib/cm.py @@ -15,7 +15,7 @@ normalization. """ -from collections.abc import Mapping, MutableMapping +from collections.abc import Mapping import numpy as np from numpy import ma @@ -52,52 +52,10 @@ def _gen_cmap_registry(): # Generate reversed cmaps. for cmap in list(cmap_d.values()): rmap = cmap.reversed() - cmap._global = True - rmap._global = True cmap_d[rmap.name] = rmap return cmap_d -class _DeprecatedCmapDictWrapper(MutableMapping): - """Dictionary mapping for deprecated _cmap_d access.""" - - def __init__(self, cmap_registry): - self._cmap_registry = cmap_registry - - def __delitem__(self, key): - self._warn_deprecated() - self._cmap_registry.__delitem__(key) - - def __getitem__(self, key): - self._warn_deprecated() - return self._cmap_registry.__getitem__(key) - - def __iter__(self): - self._warn_deprecated() - return self._cmap_registry.__iter__() - - def __len__(self): - self._warn_deprecated() - return self._cmap_registry.__len__() - - def __setitem__(self, key, val): - self._warn_deprecated() - self._cmap_registry.__setitem__(key, val) - - def get(self, key, default=None): - self._warn_deprecated() - return self._cmap_registry.get(key, default) - - def _warn_deprecated(self): - _api.warn_deprecated( - "3.3", - message="The global colormaps dictionary is no longer " - "considered public API.", - alternative="Please use register_cmap() and get_cmap() to " - "access the contents of the dictionary." - ) - - class ColormapRegistry(Mapping): r""" Container for colormaps that are known to Matplotlib by name. @@ -125,6 +83,9 @@ class ColormapRegistry(Mapping): """ def __init__(self, cmaps): self._cmaps = cmaps + self._builtin_cmaps = tuple(cmaps) + # A shim to allow register_cmap() to force an override + self._allow_override_builtin = False def __getitem__(self, item): try: @@ -177,23 +138,66 @@ def register(self, cmap, *, name=None, force=False): registered name. True supports overwriting registered colormaps other than the builtin colormaps. """ + _api.check_isinstance(colors.Colormap, cmap=cmap) + name = name or cmap.name - if name in self and not force: - raise ValueError( - f'A colormap named "{name}" is already registered.') - register_cmap(name, cmap.copy()) + if name in self: + if not force: + # don't allow registering an already existing cmap + # unless explicitly asked to + raise ValueError( + f'A colormap named "{name}" is already registered.') + elif (name in self._builtin_cmaps + and not self._allow_override_builtin): + # We don't allow overriding a builtin unless privately + # coming from register_cmap() + raise ValueError("Re-registering the builtin cmap " + f"{name!r} is not allowed.") + + # Warn that we are updating an already exisiting colormap + _api.warn_external(f"Overwriting the cmap {name!r} " + "that was already in the registry.") + + self._cmaps[name] = cmap.copy() + def unregister(self, name): + """ + Remove a colormap from the registry. + + You cannot remove built-in colormaps. + + If the named colormap is not registered, returns with no error, raises + if you try to de-register a default colormap. + + .. warning:: + + Colormap names are currently a shared namespace that may be used + by multiple packages. Use `unregister` only if you know you + have registered that name before. In particular, do not + unregister just in case to clean the name before registering a + new colormap. + + Parameters + ---------- + name : str + The name of the colormap to be removed. + + Raises + ------ + ValueError + If you try to remove a default built-in colormap. + """ + if name in self._builtin_cmaps: + raise ValueError(f"cannot unregister {name!r} which is a builtin " + "colormap.") + self._cmaps.pop(name, None) -_cmap_registry = _gen_cmap_registry() -globals().update(_cmap_registry) -# This is no longer considered public API -cmap_d = _DeprecatedCmapDictWrapper(_cmap_registry) -__builtin_cmaps = tuple(_cmap_registry) # public access to the colormaps should be via `matplotlib.colormaps`. For now, # we still create the registry here, but that should stay an implementation # detail. -_colormaps = ColormapRegistry(_cmap_registry) +_colormaps = ColormapRegistry(_gen_cmap_registry()) +globals().update(_colormaps) def register_cmap(name=None, cmap=None, *, override_builtin=False): @@ -223,14 +227,6 @@ def register_cmap(name=None, cmap=None, *, override_builtin=False): colormap. Please do not use this unless you are sure you need it. - - Notes - ----- - Registering a colormap stores a reference to the colormap object - which can currently be modified and inadvertently change the global - colormap state. This behavior is deprecated and in Matplotlib 3.5 - the registered colormap will be immutable. - """ _api.check_isinstance((str, None), name=name) if name is None: @@ -239,21 +235,12 @@ def register_cmap(name=None, cmap=None, *, override_builtin=False): except AttributeError as err: raise ValueError("Arguments must include a name or a " "Colormap") from err - if name in _cmap_registry: - if not override_builtin and name in __builtin_cmaps: - msg = f"Trying to re-register the builtin cmap {name!r}." - raise ValueError(msg) - else: - msg = f"Trying to register the cmap {name!r} which already exists." - _api.warn_external(msg) - - if not isinstance(cmap, colors.Colormap): - raise ValueError("You must pass a Colormap instance. " - f"You passed {cmap} a {type(cmap)} object.") - - cmap._global = True - _cmap_registry[name] = cmap - return + # override_builtin is allowed here for backward compatbility + # this is just a shim to enable that to work privately in + # the global ColormapRegistry + _colormaps._allow_override_builtin = override_builtin + _colormaps.register(cmap, name=name, force=override_builtin) + _colormaps._allow_override_builtin = False def get_cmap(name=None, lut=None): @@ -263,12 +250,6 @@ def get_cmap(name=None, lut=None): Colormaps added with :func:`register_cmap` take precedence over built-in colormaps. - Notes - ----- - Currently, this returns the global colormap object. This is undesired - because users could accidentally modify the global colormap. - From Matplotlib 3.6 on, this will return a copy instead. - Parameters ---------- name : `matplotlib.colors.Colormap` or str or None, default: None @@ -283,11 +264,11 @@ def get_cmap(name=None, lut=None): name = mpl.rcParams['image.cmap'] if isinstance(name, colors.Colormap): return name - _api.check_in_list(sorted(_cmap_registry), name=name) + _api.check_in_list(sorted(_colormaps), name=name) if lut is None: - return _cmap_registry[name] + return _colormaps[name] else: - return _cmap_registry[name]._resample(lut) + return _colormaps[name]._resample(lut) def unregister_cmap(name): @@ -321,14 +302,10 @@ def unregister_cmap(name): ------ ValueError If you try to de-register a default built-in colormap. - """ - if name not in _cmap_registry: - return - if name in __builtin_cmaps: - raise ValueError(f"cannot unregister {name!r} which is a builtin " - "colormap.") - return _cmap_registry.pop(name) + cmap = _colormaps.get(name, None) + _colormaps.unregister(name) + return cmap class ScalarMappable: diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.py index ed051e304405..043e9b3642ba 100644 --- a/lib/matplotlib/colors.py +++ b/lib/matplotlib/colors.py @@ -41,7 +41,6 @@ import base64 from collections.abc import Sized, Sequence, Mapping -import copy import functools import importlib import inspect @@ -646,20 +645,6 @@ def _create_lookup_table(N, data, gamma=1.0): return np.clip(lut, 0.0, 1.0) -def _warn_if_global_cmap_modified(cmap): - if getattr(cmap, '_global', False): - _api.warn_deprecated( - "3.3", - removal="3.6", - message="You are modifying the state of a globally registered " - "colormap. This has been deprecated since %(since)s and " - "%(removal)s, you will not be able to modify a " - "registered colormap in-place. To remove this warning, " - "you can make a copy of the colormap first. " - f'cmap = mpl.cm.get_cmap("{cmap.name}").copy()' - ) - - class Colormap: """ Baseclass for all scalar to RGBA mappings. @@ -776,7 +761,6 @@ def __copy__(self): cmapobject.__dict__.update(self.__dict__) if self._isinit: cmapobject._lut = np.copy(self._lut) - cmapobject._global = False return cmapobject def __eq__(self, other): @@ -798,7 +782,6 @@ def get_bad(self): def set_bad(self, color='k', alpha=None): """Set the color for masked values.""" - _warn_if_global_cmap_modified(self) self._rgba_bad = to_rgba(color, alpha) if self._isinit: self._set_extremes() @@ -811,7 +794,6 @@ def get_under(self): def set_under(self, color='k', alpha=None): """Set the color for low out-of-range values.""" - _warn_if_global_cmap_modified(self) self._rgba_under = to_rgba(color, alpha) if self._isinit: self._set_extremes() @@ -824,7 +806,6 @@ def get_over(self): def set_over(self, color='k', alpha=None): """Set the color for high out-of-range values.""" - _warn_if_global_cmap_modified(self) self._rgba_over = to_rgba(color, alpha) if self._isinit: self._set_extremes() @@ -847,7 +828,7 @@ def with_extremes(self, *, bad=None, under=None, over=None): values and, when ``norm.clip = False``, low (*under*) and high (*over*) out-of-range values, have been set accordingly. """ - new_cm = copy.copy(self) + new_cm = self.copy() new_cm.set_extremes(bad=bad, under=under, over=over) return new_cm diff --git a/lib/matplotlib/tests/test_colors.py b/lib/matplotlib/tests/test_colors.py index c9aed221108e..d1518ebd0aa3 100644 --- a/lib/matplotlib/tests/test_colors.py +++ b/lib/matplotlib/tests/test_colors.py @@ -10,7 +10,7 @@ from numpy.testing import assert_array_equal, assert_array_almost_equal -from matplotlib import _api, cbook, cm, cycler +from matplotlib import cbook, cm, cycler import matplotlib import matplotlib.colors as mcolors import matplotlib.colorbar as mcolorbar @@ -64,7 +64,7 @@ def test_resample(): def test_register_cmap(): - new_cm = copy.copy(cm.get_cmap("viridis")) + new_cm = cm.get_cmap("viridis") target = "viridis2" cm.register_cmap(target, new_cm) assert plt.get_cmap(target) == new_cm @@ -73,9 +73,6 @@ def test_register_cmap(): match="Arguments must include a name or a Colormap"): cm.register_cmap() - with pytest.warns(UserWarning): - cm.register_cmap(target, new_cm) - cm.unregister_cmap(target) with pytest.raises(ValueError, match=f'{target!r} is not a valid value for name;'): @@ -83,14 +80,13 @@ def test_register_cmap(): # test that second time is error free cm.unregister_cmap(target) - with pytest.raises(ValueError, match="You must pass a Colormap instance."): + with pytest.raises(TypeError, match="'cmap' must be"): cm.register_cmap('nome', cmap='not a cmap') def test_double_register_builtin_cmap(): name = "viridis" - match = f"Trying to re-register the builtin cmap {name!r}." - with pytest.raises(ValueError, match=match): + with pytest.raises(ValueError, match='A colormap named "viridis"'): cm.register_cmap(name, cm.get_cmap(name)) with pytest.warns(UserWarning): cm.register_cmap(name, cm.get_cmap(name), override_builtin=True) @@ -103,42 +99,6 @@ def test_unregister_builtin_cmap(): cm.unregister_cmap(name) -def test_colormap_global_set_warn(): - new_cm = plt.get_cmap('viridis') - # Store the old value so we don't override the state later on. - orig_cmap = copy.copy(new_cm) - with pytest.warns(_api.MatplotlibDeprecationWarning, - match="You are modifying the state of a globally"): - # This should warn now because we've modified the global state - new_cm.set_under('k') - - # This shouldn't warn because it is a copy - copy.copy(new_cm).set_under('b') - - # Test that registering and then modifying warns - plt.register_cmap(name='test_cm', cmap=copy.copy(orig_cmap)) - new_cm = plt.get_cmap('test_cm') - with pytest.warns(_api.MatplotlibDeprecationWarning, - match="You are modifying the state of a globally"): - # This should warn now because we've modified the global state - new_cm.set_under('k') - - # Re-register the original - with pytest.warns(UserWarning): - plt.register_cmap(cmap=orig_cmap, override_builtin=True) - - -def test_colormap_dict_deprecate(): - # Make sure we warn on get and set access into cmap_d - with pytest.warns(_api.MatplotlibDeprecationWarning, - match="The global colormaps dictionary is no longer"): - cmap = plt.cm.cmap_d['viridis'] - - with pytest.warns(_api.MatplotlibDeprecationWarning, - match="The global colormaps dictionary is no longer"): - plt.cm.cmap_d['test'] = cmap - - def test_colormap_copy(): cmap = plt.cm.Reds copied_cmap = copy.copy(cmap) diff --git a/lib/matplotlib/tests/test_pickle.py b/lib/matplotlib/tests/test_pickle.py index f4e35fb19b87..af24e4ac6fe7 100644 --- a/lib/matplotlib/tests/test_pickle.py +++ b/lib/matplotlib/tests/test_pickle.py @@ -201,7 +201,7 @@ def test_inset_and_secondary(): pickle.loads(pickle.dumps(fig)) -@pytest.mark.parametrize("cmap", cm._cmap_registry.values()) +@pytest.mark.parametrize("cmap", cm._colormaps.values()) def test_cmap(cmap): pickle.dumps(cmap) From f184145d385c432e8157b00ff99b43ca82c94f63 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Thu, 5 May 2022 18:22:08 -0400 Subject: [PATCH 2/2] TST: test re-registering builtin colormap The existing test bounced on an earlier code path. This restores missing coverage. --- lib/matplotlib/tests/test_colors.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/matplotlib/tests/test_colors.py b/lib/matplotlib/tests/test_colors.py index d1518ebd0aa3..fa30bba50a47 100644 --- a/lib/matplotlib/tests/test_colors.py +++ b/lib/matplotlib/tests/test_colors.py @@ -86,6 +86,11 @@ def test_register_cmap(): def test_double_register_builtin_cmap(): name = "viridis" + match = f"Re-registering the builtin cmap {name!r}." + with pytest.raises(ValueError, match=match): + matplotlib.colormaps.register( + cm.get_cmap(name), name=name, force=True + ) with pytest.raises(ValueError, match='A colormap named "viridis"'): cm.register_cmap(name, cm.get_cmap(name)) with pytest.warns(UserWarning):