From 4915f44b626322a1af9026e7ab51f52fb7adc8aa Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Tue, 31 Mar 2020 20:50:25 -0600 Subject: [PATCH 1/2] Warning when modifying the state of a global colormap. --- lib/matplotlib/cm.py | 29 +++++++++------ lib/matplotlib/colors.py | 19 ++++++++++ lib/matplotlib/tests/test_colors.py | 56 +++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 10 deletions(-) diff --git a/lib/matplotlib/cm.py b/lib/matplotlib/cm.py index b90e9b730150..a4c70c917a86 100644 --- a/lib/matplotlib/cm.py +++ b/lib/matplotlib/cm.py @@ -65,12 +65,16 @@ def _gen_cmap_d(): # 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 -cmap_d = _gen_cmap_d() -locals().update(cmap_d) +_cmap_d = _gen_cmap_d() +locals().update(_cmap_d) +# This is no longer considered public API +cmap_d = _cmap_d # Continue with definitions ... @@ -104,7 +108,8 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): raise ValueError("Arguments must include a name or a " "Colormap") from err if isinstance(cmap, colors.Colormap): - cmap_d[name] = cmap + cmap._global = True + _cmap_d[name] = cmap return if lut is not None or data is not None: cbook.warn_deprecated( @@ -117,7 +122,8 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): if lut is None: lut = mpl.rcParams['image.lut'] cmap = colors.LinearSegmentedColormap(name, data, lut) - cmap_d[name] = cmap + cmap._global = True + _cmap_d[name] = cmap def get_cmap(name=None, lut=None): @@ -130,9 +136,12 @@ def get_cmap(name=None, lut=None): 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*. The - default, None, means :rc:`image.cmap`. + 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 + object, which is deprecated. In Matplotlib 3.5, you will no longer be + able to modify the global colormaps in-place. + 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. @@ -141,11 +150,11 @@ def get_cmap(name=None, lut=None): name = mpl.rcParams['image.cmap'] if isinstance(name, colors.Colormap): return name - cbook._check_in_list(sorted(cmap_d), name=name) + cbook._check_in_list(sorted(_cmap_d), name=name) if lut is None: - return cmap_d[name] + return _cmap_d[name] else: - return cmap_d[name]._resample(lut) + return _cmap_d[name]._resample(lut) class ScalarMappable: diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.py index d461768d737d..bfa41d1263fd 100644 --- a/lib/matplotlib/colors.py +++ b/lib/matplotlib/colors.py @@ -484,6 +484,21 @@ def makeMappingArray(N, data, gamma=1.0): return _create_lookup_table(N, data, gamma) +def _deprecate_global_cmap(cmap): + if hasattr(cmap, '_global') and cmap._global: + cbook.warn_deprecated( + "3.3", + message="You are modifying the state of a globally registered " + "colormap. In future versions, you will not be able " + "to modify a globally registered colormap directly. " + "To eliminate this warning until then, you can make " + "a copy of the requested colormap before modifying it. ", + alternative="To modify a colormap without overwriting the " + "global state, you can make a copy of the colormap " + f"first. cmap = copy.copy(mpl.cm.get_cmap({cmap.name})" + ) + + class Colormap: """ Baseclass for all scalar to RGBA mappings. @@ -599,6 +614,7 @@ def __copy__(self): cmapobject.__dict__.update(self.__dict__) if self._isinit: cmapobject._lut = np.copy(self._lut) + cmapobject._global = False return cmapobject def set_bad(self, color='k', alpha=None): @@ -606,6 +622,7 @@ def set_bad(self, color='k', alpha=None): self._rgba_bad = to_rgba(color, alpha) if self._isinit: self._set_extremes() + _deprecate_global_cmap(self) def set_under(self, color='k', alpha=None): """ @@ -614,6 +631,7 @@ def set_under(self, color='k', alpha=None): self._rgba_under = to_rgba(color, alpha) if self._isinit: self._set_extremes() + _deprecate_global_cmap(self) def set_over(self, color='k', alpha=None): """ @@ -622,6 +640,7 @@ def set_over(self, color='k', alpha=None): self._rgba_over = to_rgba(color, alpha) if self._isinit: self._set_extremes() + _deprecate_global_cmap(self) def _set_extremes(self): if self._rgba_under: diff --git a/lib/matplotlib/tests/test_colors.py b/lib/matplotlib/tests/test_colors.py index 78f2256a3299..6e99cb71a93d 100644 --- a/lib/matplotlib/tests/test_colors.py +++ b/lib/matplotlib/tests/test_colors.py @@ -70,6 +70,62 @@ def test_register_cmap(): cm.register_cmap() +@pytest.mark.skipif(matplotlib.__version__ < "3.5", + reason="This test modifies the global state of colormaps.") +def test_colormap_global_unchanged_state(): + new_cm = plt.get_cmap('viridis') + new_cm.set_over('b') + # Make sure that this didn't mess with the original viridis cmap + assert new_cm != plt.get_cmap('viridis') + + # Also test that pyplot access doesn't mess the original up + new_cm = plt.cm.viridis + new_cm.set_over('b') + assert new_cm != plt.get_cmap('viridis') + + +@pytest.mark.skipif(matplotlib.__version__ < "3.4", + reason="This test modifies the global state of colormaps.") +def test_colormap_global_get_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) + new_cm.set_under('k') + with pytest.warns(cbook.MatplotlibDeprecationWarning, + match="You are modifying the state of a globally"): + # This should warn now because we've modified the global state + # without registering it + plt.get_cmap('viridis') + + # Test that re-registering the original cmap clears the warning + plt.register_cmap(cmap=orig_cmap) + plt.get_cmap('viridis') + + +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(cbook.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(cbook.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 + plt.register_cmap(cmap=orig_cmap) + + def test_colormap_copy(): cm = plt.cm.Reds cm_copy = copy.copy(cm) From 692b83c3f5b494e290c89148f464b95aadae52b6 Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Wed, 1 Apr 2020 17:07:05 -0600 Subject: [PATCH 2/2] Deprecation warning for cmap_d. --- .../demo_bboximage.py | 2 +- .../backends/qt_editor/figureoptions.py | 4 +- lib/matplotlib/cm.py | 81 +++++++++++++++---- lib/matplotlib/colors.py | 21 +++-- lib/matplotlib/pyplot.py | 2 +- lib/matplotlib/tests/test_colors.py | 45 +++-------- lib/matplotlib/tests/test_pickle.py | 2 +- 7 files changed, 92 insertions(+), 65 deletions(-) diff --git a/examples/images_contours_and_fields/demo_bboximage.py b/examples/images_contours_and_fields/demo_bboximage.py index 8bb4eb94426b..15f38f7bcd95 100644 --- a/examples/images_contours_and_fields/demo_bboximage.py +++ b/examples/images_contours_and_fields/demo_bboximage.py @@ -38,7 +38,7 @@ a = np.vstack((a, a)) # List of all colormaps; skip reversed colormaps. -maps = sorted(m for m in plt.cm.cmap_d if not m.endswith("_r")) +maps = sorted(m for m in plt.colormaps() if not m.endswith("_r")) ncol = 2 nrow = len(maps)//ncol + 1 diff --git a/lib/matplotlib/backends/qt_editor/figureoptions.py b/lib/matplotlib/backends/qt_editor/figureoptions.py index 962a7f94cdc3..98cf3c610773 100644 --- a/lib/matplotlib/backends/qt_editor/figureoptions.py +++ b/lib/matplotlib/backends/qt_editor/figureoptions.py @@ -140,11 +140,11 @@ def prepare_data(d, init): mappabledict[label] = mappable mappablelabels = sorted(mappabledict, key=cmp_key) mappables = [] - cmaps = [(cmap, name) for name, cmap in sorted(cm.cmap_d.items())] + cmaps = [(cmap, name) for name, cmap in sorted(cm._cmap_registry.items())] for label in mappablelabels: mappable = mappabledict[label] cmap = mappable.get_cmap() - if cmap not in cm.cmap_d.values(): + if cmap not in cm._cmap_registry.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 a4c70c917a86..11896f340a29 100644 --- a/lib/matplotlib/cm.py +++ b/lib/matplotlib/cm.py @@ -15,6 +15,7 @@ normalization. """ +from collections.abc import MutableMapping import functools import numpy as np @@ -49,7 +50,7 @@ def revcmap(data): LUTSIZE = mpl.rcParams['image.lut'] -def _gen_cmap_d(): +def _gen_cmap_registry(): """ Generate a dict mapping standard colormap names to standard colormaps, as well as the reversed colormaps. @@ -71,10 +72,50 @@ def _gen_cmap_d(): return cmap_d -_cmap_d = _gen_cmap_d() -locals().update(_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): + cbook.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." + ) + + +_cmap_registry = _gen_cmap_registry() +locals().update(_cmap_registry) # This is no longer considered public API -cmap_d = _cmap_d +cmap_d = _DeprecatedCmapDictWrapper(_cmap_registry) # Continue with definitions ... @@ -99,6 +140,13 @@ 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))``. + + Notes + ----- + Registering a colormap stores a reference to the colormap object + 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: @@ -109,7 +157,7 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): "Colormap") from err if isinstance(cmap, colors.Colormap): cmap._global = True - _cmap_d[name] = cmap + _cmap_registry[name] = cmap return if lut is not None or data is not None: cbook.warn_deprecated( @@ -123,7 +171,7 @@ def register_cmap(name=None, cmap=None, data=None, lut=None): lut = mpl.rcParams['image.lut'] cmap = colors.LinearSegmentedColormap(name, data, lut) cmap._global = True - _cmap_d[name] = cmap + _cmap_registry[name] = cmap def get_cmap(name=None, lut=None): @@ -133,15 +181,18 @@ 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, which is deprecated. + In Matplotlib 3.5, you will no longer be able to modify the global + colormaps in-place. + 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 - object, which is deprecated. In Matplotlib 3.5, you will no longer be - able to modify the global colormaps in-place. - The default, None, means :rc:`image.cmap`. + If a `.Colormap` instance, it will be returned. Otherwise, the name of + a colormap known to Matplotlib, which will be resampled by *lut*. 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. @@ -150,11 +201,11 @@ def get_cmap(name=None, lut=None): name = mpl.rcParams['image.cmap'] if isinstance(name, colors.Colormap): return name - cbook._check_in_list(sorted(_cmap_d), name=name) + cbook._check_in_list(sorted(_cmap_registry), name=name) if lut is None: - return _cmap_d[name] + return _cmap_registry[name] else: - return _cmap_d[name]._resample(lut) + return _cmap_registry[name]._resample(lut) class ScalarMappable: diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.py index bfa41d1263fd..32854abec5c2 100644 --- a/lib/matplotlib/colors.py +++ b/lib/matplotlib/colors.py @@ -484,18 +484,15 @@ def makeMappingArray(N, data, gamma=1.0): return _create_lookup_table(N, data, gamma) -def _deprecate_global_cmap(cmap): - if hasattr(cmap, '_global') and cmap._global: +def _warn_if_global_cmap_modified(cmap): + if getattr(cmap, '_global', False): cbook.warn_deprecated( "3.3", message="You are modifying the state of a globally registered " - "colormap. In future versions, you will not be able " - "to modify a globally registered colormap directly. " - "To eliminate this warning until then, you can make " - "a copy of the requested colormap before modifying it. ", - alternative="To modify a colormap without overwriting the " - "global state, you can make a copy of the colormap " - f"first. cmap = copy.copy(mpl.cm.get_cmap({cmap.name})" + "colormap. In future versions, 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()" ) @@ -619,28 +616,28 @@ def __copy__(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() - _deprecate_global_cmap(self) def set_under(self, color='k', alpha=None): """ Set the color for low out-of-range values when ``norm.clip = False``. """ + _warn_if_global_cmap_modified(self) self._rgba_under = to_rgba(color, alpha) if self._isinit: self._set_extremes() - _deprecate_global_cmap(self) def set_over(self, color='k', alpha=None): """ Set the color for high out-of-range values when ``norm.clip = False``. """ + _warn_if_global_cmap_modified(self) self._rgba_over = to_rgba(color, alpha) if self._isinit: self._set_extremes() - _deprecate_global_cmap(self) def _set_extremes(self): if self._rgba_under: diff --git a/lib/matplotlib/pyplot.py b/lib/matplotlib/pyplot.py index c4fc3f4c0e34..26a135cc34f8 100644 --- a/lib/matplotlib/pyplot.py +++ b/lib/matplotlib/pyplot.py @@ -1948,7 +1948,7 @@ def colormaps(): `_ by Carey Rappaport """ - return sorted(cm.cmap_d) + return sorted(cm._cmap_registry) def _setup_pyplot_info_docstrings(): diff --git a/lib/matplotlib/tests/test_colors.py b/lib/matplotlib/tests/test_colors.py index 6e99cb71a93d..f94cf63280e2 100644 --- a/lib/matplotlib/tests/test_colors.py +++ b/lib/matplotlib/tests/test_colors.py @@ -70,38 +70,6 @@ def test_register_cmap(): cm.register_cmap() -@pytest.mark.skipif(matplotlib.__version__ < "3.5", - reason="This test modifies the global state of colormaps.") -def test_colormap_global_unchanged_state(): - new_cm = plt.get_cmap('viridis') - new_cm.set_over('b') - # Make sure that this didn't mess with the original viridis cmap - assert new_cm != plt.get_cmap('viridis') - - # Also test that pyplot access doesn't mess the original up - new_cm = plt.cm.viridis - new_cm.set_over('b') - assert new_cm != plt.get_cmap('viridis') - - -@pytest.mark.skipif(matplotlib.__version__ < "3.4", - reason="This test modifies the global state of colormaps.") -def test_colormap_global_get_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) - new_cm.set_under('k') - with pytest.warns(cbook.MatplotlibDeprecationWarning, - match="You are modifying the state of a globally"): - # This should warn now because we've modified the global state - # without registering it - plt.get_cmap('viridis') - - # Test that re-registering the original cmap clears the warning - plt.register_cmap(cmap=orig_cmap) - plt.get_cmap('viridis') - - 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. @@ -126,6 +94,17 @@ def test_colormap_global_set_warn(): plt.register_cmap(cmap=orig_cmap) +def test_colormap_dict_deprecate(): + # Make sure we warn on get and set access into cmap_d + with pytest.warns(cbook.MatplotlibDeprecationWarning, + match="The global colormaps dictionary is no longer"): + cm = plt.cm.cmap_d['viridis'] + + with pytest.warns(cbook.MatplotlibDeprecationWarning, + match="The global colormaps dictionary is no longer"): + plt.cm.cmap_d['test'] = cm + + def test_colormap_copy(): cm = plt.cm.Reds cm_copy = copy.copy(cm) @@ -874,7 +853,7 @@ def test_pandas_iterable(pd): assert_array_equal(cm1.colors, cm2.colors) -@pytest.mark.parametrize('name', sorted(cm.cmap_d)) +@pytest.mark.parametrize('name', sorted(plt.colormaps())) def test_colormap_reversing(name): """ Check the generated _lut data of a colormap and corresponding reversed diff --git a/lib/matplotlib/tests/test_pickle.py b/lib/matplotlib/tests/test_pickle.py index b79f7476796b..6a78526b5a64 100644 --- a/lib/matplotlib/tests/test_pickle.py +++ b/lib/matplotlib/tests/test_pickle.py @@ -190,7 +190,7 @@ def test_shared(): assert fig.axes[1].get_xlim() == (10, 20) -@pytest.mark.parametrize("cmap", cm.cmap_d.values()) +@pytest.mark.parametrize("cmap", cm._cmap_registry.values()) def test_cmap(cmap): pickle.dumps(cmap)