From a8998117a2771af2f28cb508b114f00ee7a32e7c Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Wed, 23 Sep 2020 23:07:22 -0400 Subject: [PATCH 1/3] FIX: return static view of good/bad/under Otherwise we will hand a view back to the user and it will change under them if `set_under` (e.g.) is called. --- lib/matplotlib/colors.py | 6 +++--- lib/matplotlib/tests/test_colors.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.py index 3e8226b0ce25..9500e5234ff1 100644 --- a/lib/matplotlib/colors.py +++ b/lib/matplotlib/colors.py @@ -652,7 +652,7 @@ def get_bad(self): """Get the color for masked values.""" if not self._isinit: self._init() - return self._lut[self._i_bad] + return np.array(self._lut[self._i_bad]) def set_bad(self, color='k', alpha=None): """Set the color for masked values.""" @@ -665,7 +665,7 @@ def get_under(self): """Get the color for low out-of-range values.""" if not self._isinit: self._init() - return self._lut[self._i_under] + return np.array(self._lut[self._i_under]) def set_under(self, color='k', alpha=None): """Set the color for low out-of-range values.""" @@ -678,7 +678,7 @@ def get_over(self): """Get the color for high out-of-range values.""" if not self._isinit: self._init() - return self._lut[self._i_over] + return np.array(self._lut[self._i_over]) def set_over(self, color='k', alpha=None): """Set the color for high out-of-range values.""" diff --git a/lib/matplotlib/tests/test_colors.py b/lib/matplotlib/tests/test_colors.py index d180fb28afa5..0de33f0e77f0 100644 --- a/lib/matplotlib/tests/test_colors.py +++ b/lib/matplotlib/tests/test_colors.py @@ -1187,6 +1187,16 @@ def test_get_under_over_bad(): assert_array_equal(cmap.get_bad(), cmap(np.nan)) +@pytest.mark.parametrize('kind', ('over', 'under', 'bad')) +def test_non_mutable_get_values(kind): + cmap = copy.copy(plt.get_cmap('viridis')) + init_value = getattr(cmap, f'get_{kind}')() + getattr(cmap, f'set_{kind}')('k') + black_value = getattr(cmap, f'get_{kind}')() + assert np.all(black_value == [0, 0, 0, 1]) + assert not np.all(init_value == black_value) + + def test_colormap_alpha_array(): cmap = plt.get_cmap('viridis') vals = [-1, 0.5, 2] # under, valid, over From d28cbf2b265252441770293c9df25297cf5a5ba0 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Wed, 23 Sep 2020 22:11:45 -0400 Subject: [PATCH 2/3] API: remove implicit colormap generation in register_cmap This was deprecated in 3.3 via #15875 Enforce type of cmap as Colormap because we may not know that the object registered is of the wrong type until well after it was registered, defensively check here. We could do a duck-type check, but this is much simpler and we can cross the bridge of writing a duck-type checking function when someone has a use case for registering compatible-but-not-derived-from-Colormap instances. --- lib/matplotlib/cm.py | 46 ++++++++++++----------------- lib/matplotlib/tests/test_colors.py | 3 ++ 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/lib/matplotlib/cm.py b/lib/matplotlib/cm.py index b8a67550bff3..a03c40541b97 100644 --- a/lib/matplotlib/cm.py +++ b/lib/matplotlib/cm.py @@ -100,25 +100,27 @@ def _warn_deprecated(self): # Continue with definitions ... -def register_cmap(name=None, cmap=None, data=None, lut=None): +def register_cmap(name=None, cmap=None): """ Add a colormap to the set recognized by :func:`get_cmap`. - It can be used in two ways:: + Register a new colormap to be accessed by name :: - register_cmap(name='swirly', cmap=swirly_cmap) + LinearSegmentedColormap('swirly', data, lut) + register_cmap(cmap=swirly_cmap) - register_cmap(name='choppy', data=choppydata, lut=128) + Parameters + ---------- + name : str, optional + The name that can be used in :func:`get_cmap` or :rc:`image.cmap` - In the first case, *cmap* must be a :class:`matplotlib.colors.Colormap` - instance. The *name* is optional; if absent, the name will - be the :attr:`~matplotlib.colors.Colormap.name` attribute of the *cmap*. + If absent, the name will be the :attr:`~matplotlib.colors.Colormap.name` + attribute of the *cmap*. + + cmap : matplotlib.colors.Colormap + Despite being the second argument and having a default value, this + is a required argument. - The second case is deprecated. Here, the three arguments are passed to - the :class:`~matplotlib.colors.LinearSegmentedColormap` initializer, - 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))``. Notes ----- @@ -134,23 +136,13 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): except AttributeError as err: raise ValueError("Arguments must include a name or a " "Colormap") from err - if isinstance(cmap, colors.Colormap): - cmap._global = True - _cmap_registry[name] = cmap - return - if lut is not None or data is not None: - cbook.warn_deprecated( - "3.3", - message="Passing raw data via parameters data and lut to " - "register_cmap() is deprecated since %(since)s and will " - "become an error %(removal)s. Instead use: register_cmap(" - "cmap=LinearSegmentedColormap(name, data, lut))") - # For the remainder, let exceptions propagate. - if lut is None: - lut = mpl.rcParams['image.lut'] - cmap = colors.LinearSegmentedColormap(name, data, lut) + + 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 def get_cmap(name=None, lut=None): diff --git a/lib/matplotlib/tests/test_colors.py b/lib/matplotlib/tests/test_colors.py index 0de33f0e77f0..51de61b29fa9 100644 --- a/lib/matplotlib/tests/test_colors.py +++ b/lib/matplotlib/tests/test_colors.py @@ -72,6 +72,9 @@ def test_register_cmap(): match='Arguments must include a name or a Colormap'): cm.register_cmap() + with pytest.raises(ValueError, match="You must pass a Colormap instance."): + cm.register_cmap('nome', cmap='not a cmap') + def test_colormap_global_set_warn(): new_cm = plt.get_cmap('viridis') From 1a9dacff1cb0be7bf2f7bc765bb0df6dadcb98ac Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Thu, 22 Aug 2019 19:26:42 -0400 Subject: [PATCH 3/3] ENH/API: be more defensive in cm.register_cmap + deregister - raise if you try to over write a default colormap - warn if you try to over write a user-added colormap - add method to de-register a color map - add escape hatch to force re-registering builtin colormaps Co-Authored-By: David Stansby --- .../next_api_changes/behavior/15127-TAC.rst | 8 +++ doc/users/next_whats_new/2019-08_tac.rst | 6 ++ lib/matplotlib/cm.py | 61 ++++++++++++++++++- lib/matplotlib/tests/test_colors.py | 38 ++++++++++-- 4 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 doc/api/next_api_changes/behavior/15127-TAC.rst create mode 100644 doc/users/next_whats_new/2019-08_tac.rst diff --git a/doc/api/next_api_changes/behavior/15127-TAC.rst b/doc/api/next_api_changes/behavior/15127-TAC.rst new file mode 100644 index 000000000000..fd68c0150551 --- /dev/null +++ b/doc/api/next_api_changes/behavior/15127-TAC.rst @@ -0,0 +1,8 @@ +Raise or warn on registering a colormap twice +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When using `matplotlib.cm.register_cmap` to register a user provided +or third-party colormap it will now raise a `ValueError` if trying to +over-write one of the built in colormaps and warn if trying to over +write a user registered colormap. This may raise for user-registered +colormaps in the future. diff --git a/doc/users/next_whats_new/2019-08_tac.rst b/doc/users/next_whats_new/2019-08_tac.rst new file mode 100644 index 000000000000..3e7e3648b421 --- /dev/null +++ b/doc/users/next_whats_new/2019-08_tac.rst @@ -0,0 +1,6 @@ + +Add ``cm.unregister_cmap`` function +----------------------------------- + +`.cm.unregister_cmap` allows users to remove a colormap that they +have previously registered. diff --git a/lib/matplotlib/cm.py b/lib/matplotlib/cm.py index a03c40541b97..d4a8b1df5cb2 100644 --- a/lib/matplotlib/cm.py +++ b/lib/matplotlib/cm.py @@ -24,6 +24,7 @@ from matplotlib import _api, colors, cbook from matplotlib._cm import datad from matplotlib._cm_listed import cmaps as cmaps_listed +from matplotlib.cbook import _warn_external LUTSIZE = mpl.rcParams['image.lut'] @@ -95,12 +96,12 @@ def _warn_deprecated(self): locals().update(_cmap_registry) # This is no longer considered public API cmap_d = _DeprecatedCmapDictWrapper(_cmap_registry) - +__builtin_cmaps = tuple(_cmap_registry) # Continue with definitions ... -def register_cmap(name=None, cmap=None): +def register_cmap(name=None, cmap=None, *, override_builtin=False): """ Add a colormap to the set recognized by :func:`get_cmap`. @@ -121,6 +122,12 @@ def register_cmap(name=None, cmap=None): Despite being the second argument and having a default value, this is a required argument. + override_builtin : bool + + Allow built-in colormaps to be overridden by a user-supplied + colormap. + + Please do not use this unless you are sure you need it. Notes ----- @@ -128,6 +135,7 @@ def register_cmap(name=None, cmap=None): which can currently be modified and inadvertantly change the global colormap state. This behavior is deprecated and in Matplotlib 3.5 the registered colormap will be immutable. + """ cbook._check_isinstance((str, None), name=name) if name is None: @@ -136,10 +144,18 @@ def register_cmap(name=None, cmap=None): 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." + _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 @@ -179,6 +195,47 @@ def get_cmap(name=None, lut=None): return _cmap_registry[name]._resample(lut) +def unregister_cmap(name): + """ + Remove a colormap recognized by :func:`get_cmap`. + + You may not 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_cmap` 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 un-registered + + Returns + ------- + ColorMap or None + If the colormap was registered, return it if not return `None` + + Raises + ------ + 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) + + class ScalarMappable: """ A mixin class to map scalar data to RGBA. diff --git a/lib/matplotlib/tests/test_colors.py b/lib/matplotlib/tests/test_colors.py index 51de61b29fa9..8a1a96b47389 100644 --- a/lib/matplotlib/tests/test_colors.py +++ b/lib/matplotlib/tests/test_colors.py @@ -64,18 +64,45 @@ def test_resample(): def test_register_cmap(): - new_cm = copy.copy(plt.cm.viridis) - cm.register_cmap('viridis2', new_cm) - assert plt.get_cmap('viridis2') == new_cm + new_cm = copy.copy(cm.get_cmap("viridis")) + target = "viridis2" + cm.register_cmap(target, new_cm) + assert plt.get_cmap(target) == new_cm with pytest.raises(ValueError, - match='Arguments must include a name or a Colormap'): + 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;'): + cm.get_cmap(target) + # test that second time is error free + cm.unregister_cmap(target) + with pytest.raises(ValueError, match="You must pass a Colormap instance."): 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): + cm.register_cmap(name, cm.get_cmap(name)) + with pytest.warns(UserWarning): + cm.register_cmap(name, cm.get_cmap(name), override_builtin=True) + + +def test_unregister_builtin_cmap(): + name = "viridis" + match = f'cannot unregister {name!r} which is a builtin colormap.' + with pytest.raises(ValueError, match=match): + 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. @@ -97,7 +124,8 @@ def test_colormap_global_set_warn(): new_cm.set_under('k') # Re-register the original - plt.register_cmap(cmap=orig_cmap) + with pytest.warns(UserWarning): + plt.register_cmap(cmap=orig_cmap, override_builtin=True) def test_colormap_dict_deprecate():