Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Begin warning on modifying global state of colormaps#16991

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

Merged
timhoffm merged 2 commits intomatplotlib:masterfromgreglucas:get_cmap_warn
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletionexamples/images_contours_and_fields/demo_bboximage.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"))
Copy link
Member

@timhoffmtimhoffmApr 26, 2020
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Just as a remark (not necessarily to be acted upon in this PR):

By making the cmap registry private, we don't have an official way to get the registered colormaps from thecm module, where I would expect such functionality. This is something we should take care of in followup PRs.

I'm not even sureplt.colormaps() is needed. First, It's essentially a lengthy description of colormaps, which should be somewhere in the docs but not in a pyplot docstring. Second, I don't see providing the colormaps as a core functionality of pyplot, but since there's a lot of historic colormap stuff in pyplot, that's maybe ok.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I had the same thought:#16991 (comment)

I'm not sure what the best step is to expose the access, but would agree that I don't want to have to go to pyplot if not necessary. Maybe rather than acmap_d, we could do something like your MutableMapping suggestion and then override the get/set methods to ensure copy/immutable return.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Sorry if this is a bit unstructured. I just stated typing and ideas unfolded along the way.

There would be two ways:

  1. Thecm module currently uses simple functionsget_cmap() andregister_cmap(). In that spirit, you could add a functionget_cmap_names() (naming t.b.d.) but we have to make a distinction betweenget_cmap returning an instance and that function (probably?) returning strings.

  2. The alternative would be to expose aColormapRegistry object, which provides all necessary functionality. That would likely be subclassingMutableMapping to look dict-like but still perform all relevant checks (maybe it would alternatively only implementMapping and provide an explicit register method if we feel the mutation should be more special (i.e. we don't wantdel registry['jet'] orregistry['jet'] = Colormap(...)).
    This would also replace the module level functions; at least their implementation. Whether they will additionally stay for backward compatibility is a separate topic.

In the end 2 is the more complex approach. However, I have a slight preference for it because it properly encapsulates the colormap handling. On the downside it changes the API from calling simple cm functions to an object. But then again, we could move the actual instance to a variablematplotlib.colormaps, which may make more sense and users would usually not need to import matplotlib.cm anymore; andpyplot.colormaps could later even point to the same instance (with some API compatibility because the keys of that mapping would then colormap names, sofor name in plt.colormaps() would still work.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I agree with pretty much everything you're saying. I also don't think 2 would be that hard to implement. An issue with naming will be found with:#11413 which is already proposing to rename cm to colormaps. This discussion is really more a part of the MEP:https://matplotlib.org/3.2.1/devel/MEP/MEP21.html
Maybe we should do some larger work on an overarching vision there and then decide on how best to get there?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

If we provide colormap registry access frommatplotlib directly,cm.get_cmap andcm.register_cmap become obsolete. There won't be a need for the end user to importcm, which in turn reduces the need for renaming (#11413). But we could still find other names for that.

It would be reasonable to detail the overall vision.


ncol = 2
nrow = len(maps)//ncol + 1
Expand Down
4 changes: 2 additions & 2 deletionslib/matplotlib/backends/qt_editor/figureoptions.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = [
Expand Down
80 changes: 70 additions & 10 deletionslib/matplotlib/cm.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,7 @@
normalization.
"""

from collections.abc import MutableMapping
import functools

import numpy as np
Expand DownExpand Up@@ -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.
Expand All@@ -65,12 +66,56 @@ 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)
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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This does not warn forlist(mpl.cm.cmap_d) but does warn forlist(mlp.cm.cmap_d.items()). Was this an oversight that__iter__ does not warn or a decision that because it is only giving a list of keys that could then go toget_cmap it should not wark?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Oversight, the iter still warns because of the getitem, but I didn't think of people just listing the colormaps and using it manually themselves elsewhere. I just added it here as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

same for__len__ below? Sorry for the thousand paper cuts at the end...

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

No worries, easy enough to fix. Should have just done it myself ;)

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 = _DeprecatedCmapDictWrapper(_cmap_registry)


# Continue with definitions ...
Expand All@@ -95,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:
Expand All@@ -104,7 +156,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Please document in the docstring that if passing a Colormap instance, that instance will become immutable.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I added in a Notes section to both theget_cmap andregister_cmap functions now. Let me know if you want any different wording there. It may be odd to talk about future behavior there considering you can't do anything to get that future behavior yet? But I can't think of any great wording off the top of my head.

_cmap_registry[name] = cmap
return
if lut is not None or data is not None:
cbook.warn_deprecated(
Expand All@@ -117,7 +170,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_registry[name] = cmap


def get_cmap(name=None, lut=None):
Expand All@@ -127,11 +181,17 @@ 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*.The
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
Expand All@@ -141,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:
returncmap_d[name]
return_cmap_registry[name]
else:
returncmap_d[name]._resample(lut)
return_cmap_registry[name]._resample(lut)


class ScalarMappable:
Expand Down
16 changes: 16 additions & 0 deletionslib/matplotlib/colors.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -484,6 +484,18 @@ def makeMappingArray(N, data, gamma=1.0):
return _create_lookup_table(N, data, gamma)


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 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.
Expand DownExpand Up@@ -599,10 +611,12 @@ 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):
"""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()
Expand All@@ -611,6 +625,7 @@ 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()
Expand All@@ -619,6 +634,7 @@ 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()
Expand Down
2 changes: 1 addition & 1 deletionlib/matplotlib/pyplot.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1948,7 +1948,7 @@ def colormaps():
<https://www.mathworks.com/matlabcentral/fileexchange/2662-cmrmap-m>`_
by Carey Rappaport
"""
return sorted(cm.cmap_d)
return sorted(cm._cmap_registry)


def _setup_pyplot_info_docstrings():
Expand Down
37 changes: 36 additions & 1 deletionlib/matplotlib/tests/test_colors.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -70,6 +70,41 @@ def test_register_cmap():
cm.register_cmap()


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_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)
Expand DownExpand Up@@ -818,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
Expand Down
2 changes: 1 addition & 1 deletionlib/matplotlib/tests/test_pickle.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)

Expand Down

[8]ページ先頭

©2009-2025 Movatter.jp