@timhoffm, just wanted to add you here to get your feedback since I can't explicitly request you as a reviewer. Happy to hear your thoughts on this.

Actually, I started searching around a bit to see where/how others are using cmap_d and I think deprecating cmap_d needs a little more careful consideration for a good alternative/replacement. There really is no way to access a list of what all is in the cmap_d dictionary explicitly without either using cmap_d or to use the pyplot interface. We should think about adding a get_registered_cmaps() function or similar to list what is available to the get_cmap() call.

We explicitly use it in this example:
https://matplotlib.org/3.1.0/gallery/images_contours_and_fields/demo_bboximage.html

There is this SO post with the first answer stating to use sorted(cmap_d):
https://stackoverflow.com/questions/47302343/what-names-can-be-used-in-plt-cm-get-cmap

There is also mention of accessing plt.cm.datad! So, the plt.cm registering all public methods is going to be tricky.

Maybe removal should be scheduled for 4.0 or whatever sounds like "a long time from now".

@timhoffm, Did you have any thoughts on this? I'd like to try to get some of the warnings in for the 3.3 release to get the deprecation cycle started. No API code is being changed yet.

• The first commit is warning on set/get from the global cmap registry and I think everyone was in agreement about that.
• The second commit is warning on accessing cmap_d directly, which was a little harder to warn against using (Discussion above about potentially waiting until 3.7 only for a dependency).

👍 for pushing the deprecations into 3.3.

Thanks, Tim! Great suggestions. I just pushed up a commit with all of those suggestions in it.

I also decided to take your advice for more descriptive names and apply it to the private _cmap_d that I added in. It is now called _cmap_registry. I can change that back if desired, but figure this will help to describe what the dictionary actually is.

I labeled as release critical as I think this is very close and do not want it to fall through the cracks as we do the final push to 3.3.

👍
If anyone has ideas on why the docs are failing, let me know... I tried to look for what the Error was and couldn't figure out how to parse the Sphinx output for it. It must be something with docstring formatting?

Good comments on the rest of it. I'll try and push up changes to address them tomorrow.

Can you try rebasing on current master?

We are very cranky about making sure all of the internal references actually work (with a list of allowed errors). Sphinx made some internal changes that affected how references to the nested classes work so we re-generated the allowed list to the new version. Unlike travis which runs against the shadow merge commit GH makes to determine if your branch is mergable, circle runs against your actually commit so if you have not rebased on master from when we fixed that circle will be broken.

Just wanted to say, bravo for the super clear deprecation message including specific code suggestions for how to update downstream code. I can see that took extra effort to put together and it's much appreciated. 👏

I wish I'd followed this better. I did

cmap = cm.RdBu_r
cmap.set_over('yellow')

and was told I had to do:

MatplotlibDeprecationWarning: 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. cmap = copy.copy(mpl.cm.get_cmap("RdBu_r"))

Thats pretty user hostile. When will this get fixed?

I guess it depends on what you mean by fixed.
#18503 will return a copy of the colormap, which should remove a lot of the Deprecation dance. That will hopefully be in 3.4...

We could update the message pretty easily too if you have any suggestions there.

I guess I mean an alternative that doesn't require an import and copy call. If that is #18503 we should get that in ASAP.

I don't understand why a deprecation was deemed necessary for that PR to go in though. I'd actually vote to revert this for V3.3.3, but maybe I'm not understanding some subtlety.

This was to enable people who may have been (ab)using the ability to modify the global colormap to change before we remove the capability entirely.

In your example, if you keep running the program, or go to some other Jupyter notebook cell and want to get the RdBu colormap again, it would still have the yellow over color, which is probably not desired.

cmap = cm.RdBu_r
cmap.set_over('yellow')

# Later on you get the built-in colormap again
cmap2 = cm.get_cmap('RdBu_r')
cmap2(np.inf)  # yellow

I agree it is somewhat annoying to import copy for now. We had discussed bringing in a copy=True keyword argument, but then we'd just have to deprecate that after a single release once we always return a copy/immutable version of the colormap.

Sure, I agree with the over all strategy, just not the tactics. As of 3.3 there is no other way to get the colormap except to do copy.copy. So now our users who need this, change to copy.copy. And then in 3.4 we offer a way to return the colormap, so now those users have to change again. Why didn't we just introduce the change?

As of 3.3 there is no other way to get the colormap except to do copy.copy

Maybe this is just a poorly worded message then. The only reason to use copy here is if you need to re-get the original RdBu colormap at some later time. You can certainly keep using the cm.RdBu_r throughout your code as you normally would, and the warnings should only happen once when you are setting the over/under/bad on a built-in colormap.

My vote would be to leave this as is for now and make sure 3.4 is returning a copy. I really don't think this is that egregious, more of a minor annoyance along the deprecation path.

fwiw I guess #14645 may help too?

The message would be useful if people had to change their code. But so far as I can tell we are going to allow cmap=cm.RdBu_r will in the future return a copy, so there is no code we need to warn the user to change.

The only users who would have to change behaviour are those who were for some reason depending on being able to access the colormap from the global state.

cmap = cm.RdBu
cmap.set_over_color([0, 1., 1., 1.])

.....
cmap=cm.RdBu
assert cmap.get_over_color() == [0, 1., 1., .1.]

which I imagine is exceedingly rare. The rest get a misleading Deprecation message that makes it sound like they won't be able to call set_over_color on the object returns by cm.RdBu any more.

Did we decide to go down the route of module properties (which is py37 so we have it on main)? I do not remember any discussion of it and am not finding it in the text of this issue.

If we do not, then we can't make cmap = cm.RdBu to return a copy as it is Python assignment and we don't have a hook to make the copy.

Good question. I think that was brought up as a possibility and an alternative would be to make the cmap object's immutable (Tim's proposal). I think we could get away with something like defining our own getattr at the module level for the "return a copy" case:

def __getattr__(name):
    return ColormapRegistry[name]

All of this discussion leads me to think we should really have a clear end-goal in mind and lay that out. Then, we can start shimming the deprecations in. There is already MEP21, which looks to be stagnant. Mind if I revive that MEP with some new text and then we can start linking issues/fixes to a more central location?

... or use the Projects feature...

Making attribute access return new (mutable) copies seems rather contrary to standard python semantics. In particular this means that something like

from matplotlib import cm
cm.jet.set_bad(...)
plt.imshow(..., cmap=cm.jet)

would silently "fail", whereas

from matplotlib.cm import jet
jet.set_bad(...)
plt.imshow(..., cmap=jet)

would succeed... but I doubt that having both snippets behave differently is a good idea.
(Unless I've completely misunderstood the idea here.)

I agree with @anntzer , while cute there are too many weird cases. Just because you can hijack and change something in Python does not mean you should.

We need to decide if we only want to deprecate mutating the global state when getting the colormap through get_cmap and let people continue to mutate global state with e.g. cm.viridis (which then means we will have to decide if get_cmap('foo') returns a fresh copy of the default state (which would make cm.viridis and `cm.get_cmap('viridis') decoupled) or a copy of the currently accessible one at the module level (which would defeat the purpose of trying to protect the user from spooky action-at-a-distance)) or make the attribute access colormaps actually immutable (which will break a bunch of people).

I think when we wrote the warning we were not thinking about the attribute access case at all. Do we document / recommend that usage anywhere?

I find it useful in these cases to think about what the ideal situation would be and only then figure out how we would get there from where we are (if possible). So, how would this look if we were to start over?

My 50c would be that we should just make all colormaps immutable (after merging the with_extremes PR (or similar functionality) and possibly with a very long deprecation period, given that we are basically breaking everyone who way setting over/under/bad before with essentially no way to write code that compatible both with old and new Matplotlib versions, as with_extremes did not exist before).

Especially if we are going to push in the direction of encouraging the use of the global cmap registry (via register_cmap, which I didn't particularly like but I guess that discussion is closed), any colormap can potentially become global state, e.g. via

# in foo.py
foo_cmap = LinearSegmentedColormap(...)
register_cmap("foo", foo_cmap)
...
# in module2.py
imshow(..., cmap="foo")
...
# in module3.py
from foo import foo_cmap
foo_cmap.set_over(...)  # oops, this will affect module2.py if run before it.

It seems to me that the original problem was defining globals like cm.jet in the first place. If we had just stuck with jet=cm.get_cmap('jet') then there would not be a problem.

Given the state we are in, it seems sensible to me that if you modify a global like cm.jet (i.e. cm.jet.set_over) you get what you asked for, and cm.jet is changed everywhere. However, if you call jet=cm.get_cmap('jet') you should get a local copy that does not affect the global state.

It would be nice if jet = cm.jet also returned a copy. Is it even possible to make that return a copy?

TLDR: it would be nice if we could return copies whenever possible, because I don't think it is at all clear to users that cm.jet is a global.

Actually, now that I look at it again, I guess a solution for the "sneaky global" problem I mentioned just above would be for register_cmap to be in charge of registering a copy of the colormap, so that later changes to the colormap passed as parameter to register_cmap don't affect the global registry. I think this is similar to how when you do y = [...]; plt.plot(y), later changes to y don't affect the plotted line.

@anntzer it's rarely the problem that one registers a colormap instance and then modifies the instance, thus modifying the registered colormap. While that's a theoretical issue I'm not seriously worried about that. Few people register colormaps themselves, and even fewer will hold on to instances some times and use the registry other times.

The far more common problem is obtaining a reference to the registered colormap and modifying that, without knowing that it affects the registered state.

I'd guess making a copy at registration time is simple enough to be worth considering, but won't insist on that either.

The module attribute access has been with us from at least 2004 (e34a333). I think clawing those back is off the table.

So if we were to make the attribute access return a copy:

Pro: same behavior as get_cmap
Con: very surprising, leads to cases where the code looks right, but does not do what it expects

I think the case of

cm.viridis.set_under('w')  # expect to mutate in place
ax.imshow(..., cmap=cm.viridis)  # does not work!!

is enough to take that option out of consideration (even if we could make it work)

If we back off making the attribute access immutable:

Pro: leave the foot-cannon that people are using in place
Con: but the foot-cannon will only half work as things like

cm.jet.set_under('k')
ax.imshow(..., 'jet')

which is a variation on the issue with making cm.jet return a copy. The things that make me angriest when using APIs are when it runs, does not do what I want, but the most "obvious" (and yes I know "obvious" is super subjective) mental model of how it works inside says it should do what I want.

It also will make cm.<name> and cm.get_cmap(name) give possibly different results which seems too confusing to do

If we make the attribute access immutable (which is the status quo)

Pro: avoids all of the cons above
Con: will require a wider net of changes than we discussed going into this.

Backing off of making the global versions of the cmap objects immutable is also an option, but I think that the action-at-a-distance and general global state issues are worth cleaning up for all of the reasons we had when we started this.

On net, I think we should go with making the attribute access immutable and not try to return a copy. It may not be ideal, but I think it is the least-bad of the options we have.

I agree about the copy in cmap_register (I checked, we do not currently do that but we do flag them as global).

It!'s reasonable to do. I'm just clarifying that it does not solve our main issue.

At some point @efiring wanted to change the name of this module anyhow. Is yet another possibility just writing a new way to do this from scratch and leaving cm the way it is, and soft-deprecating it?