Skip to content

Add a dedicated ColormapRegistry class #18503

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 15, 2021
Merged

Add a dedicated ColormapRegistry class #18503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40f4013
Add a dedicated ColormapRegistry class
timhoffm Sep 16, 2020
3f4ab7f
Update doc/users/next_whats_new/colormaps.rst
timhoffm Aug 14, 2021
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 deletion .flake8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ per-file-ignores =
setupext.py: E501
tests.py: F401

lib/matplotlib/__init__.py: F401
lib/matplotlib/__init__.py: E402, F401
lib/matplotlib/_api/__init__.py: F401
lib/matplotlib/_cm.py: E122, E202, E203, E302
lib/matplotlib/_mathtext.py: E221, E251
Expand Down
6 changes: 6 additions & 0 deletions doc/api/matplotlib_configuration_api.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ Logging

.. autofunction:: set_loglevel

Colormaps
=========

.. autodata:: colormaps
:no-value:

Miscellaneous
=============

Expand Down
18 changes: 18 additions & 0 deletions doc/users/next_whats_new/colormaps.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Colormap registry
------------------

Colormaps are now managed via `matplotlib.colormaps`, which is a
`.ColormapRegistry`.

Colormaps can be obtained using item access::

import matplotlib as mpl
cmap = mpl.colormaps['viridis']

To register new colormaps use::

mpl.colormaps.register(my_colormap)

The use of `matplotlib.cm.get_cmap` and `matplotlib.cm.register_cmap` is
discouraged in favor of the above. Within `.pyplot` the use of
``plt.get_cmap()`` and ``plt.register_cmap()`` will continue to be supported.
5 changes: 5 additions & 0 deletions lib/matplotlib/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1445,3 +1445,8 @@ def inner(ax, *args, data=None, **kwargs):
_log.debug('interactive is %s', is_interactive())
_log.debug('platform is %s', sys.platform)
_log.debug('loaded modules: %s', list(sys.modules))


# workaround: we must defer colormaps import to after loading rcParams, because
# colormap creation depends on rcParams
from matplotlib.cm import _colormaps as colormaps
Copy link
Member

Choose a reason for hiding this comment

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

Could we handle this in the module level __getattr__ now that we have than in place?

77 changes: 75 additions & 2 deletions lib/matplotlib/cm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
normalization.
"""

from collections.abc import MutableMapping
from collections.abc import Mapping, MutableMapping

import numpy as np
from numpy import ma
Expand Down Expand Up @@ -91,13 +91,86 @@ def _warn_deprecated(self):
)


class ColormapRegistry(Mapping):
r"""
Container for colormaps that are known to Matplotlib by name.

The universal registry instance is `matplotlib.colormaps`. There should be
no need for users to instantiate `.ColormapRegistry` themselves.

Read access uses a dict-like interface mapping names to `.Colormap`\s::

import matplotlib as mpl
cmap = mpl.colormaps['viridis']

Returned `.Colormap`\s are copies, so that their modification does not
change the global definition of the colormap.

Additional colormaps can be added via `.ColormapRegistry.register`::

mpl.colormaps.register(my_colormap)
"""
def __init__(self, cmaps):
self._cmaps = cmaps
Copy link
Member

Choose a reason for hiding this comment

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

makes sense why to do this as to wrap this around the existing dict, but I am wary of sharing the underlying storage this way.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's very intentional, that the registry shares the underlying storage: For a transition period, the registry and the cmap functions will coexists. Due to API quirks in the functions, it's better if both can directly work on the underlying storage and not one API calling the orther.

If you only have concerns with the constructor accepting a prebuilt mapping, we can change it to creating an empty registry by default and have a private ColormapRegistry._from_dict() that we use to inject the existing dict.

Copy link
Member

Choose a reason for hiding this comment

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

I would be happy warning in the init docstring saying we keep a reference to the input dict.

It makes total sense why you are doing it this way, just set off alarm bells in my head (we have had some cases at BNL where we did not mean to do this and ended up with hard-to-debug action-at-a-distance bugs!).


def __getitem__(self, item):
try:
return self._cmaps[item].copy()
except KeyError:
raise KeyError(f"{item!r} is not a known colormap name")

def __iter__(self):
return iter(self._cmaps)

def __len__(self):
return len(self._cmaps)

def __str__(self):
return ('ColormapRegistry; available colormaps:\n' +
', '.join(f"'{name}'" for name in self))

def register(self, cmap, *, name=None, force=False):
Copy link
Member

Choose a reason for hiding this comment

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

I think we need an deregister as well? with force you can over-write an existing colormap, but I still think we will want a way to remove a key.

I do not think we want to promote this to a MutableMapping and support the full set of del / pop / clear. The act of adding and removing color maps is a different activity than the act of getting a colormap out of the registry.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that some sort of deregister should be there at some point. Details would need to be discussed, e.g. if you can deregister built-in cmaps (I think that should not be possible these are reseverd names with fixed meaning and no public API should change them).

Anyway, we currently don't have a deregister function, so there is no hurry with adding that new feature in the initial version of the registry.

Copy link
Member

Choose a reason for hiding this comment

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

We have unregister_cmap which went out in 3.4 via #15127

matplotlib/lib/matplotlib/cm.py

Lines 197 to 199 in 1761822

def unregister_cmap(name):
"""
Remove a colormap recognized by :func:`get_cmap`.

"""
Register a new colormap.

The colormap name can then be used as a string argument to any ``cmap``
parameter in Matplotlib. It is also available in ``pyplot.get_cmap``.

The colormap registry stores a copy of the given colormap, so that
future changes to the original colormap instance do not affect the
registered colormap. Think of this as the registry taking a snapshot
of the colormap at registration.

Parameters
----------
cmap : matplotlib.colors.Colormap
The colormap to register.

name : str, optional
The name for the colormap. If not given, ``cmap.name`` is used.

force: bool, default: False
If False, a ValueError is raised if trying to overwrite an already
registered name. True supports overwriting registered colormaps
other than the builtin colormaps.
"""
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())


_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)

# Continue with definitions ...
# public acces 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)


def register_cmap(name=None, cmap=None, *, override_builtin=False):
Expand Down