DeprecationWarning when changing color maps

Summary

In v3.4.0rc1, calls to a colormap's set_bad, set_under, and set_over functions now trigger a deprecation warning saying that modifying a registered color map will soon be disallowed. This seems overly protective. If users have called any of these routines, then they are presumably doing so intentionally, probably after reading the documentation. This seems like the kind of routine customization that any user might expect to perform without going through the hoop of copying a color map every time.

Proposed fix

Remove the calls to _warn_if_global_cmap_modified(self) in the Colormap.set_bad, set_under, and set_over functions in color.py and change future plans to disallow such calls.

Didn't 3.3 also have this behaviour?

This came in on #16991 with lots of discussion of this exact point. I actually do not feel that discussion was adequately resolved, so I guess we should rehash it.

In my opinion, I disagree with the idea of making the global unmutable. I think if you modify a global, you meant to modify it, and that we should provide a method to return a copy. The original idea of #16991 was that if you change the "over" color of the global cm.jet you don't want that change to propagate forward, but I don't see any reason that was deemed to be true. If we are going to deprecate anything, it should be access to the global to reduce implicit state.

I have been dealing with a number of additional warnings that I only noticed when updating my package to use v3.4.0c1. It is possible this also occurred in v3.3 and I apologize for not spending more time understanding the history of this change before posting. It seems as if part of the debate concerns internal plumbing that I am not familiar with. I got the impression that a call to im.cmap.set_bad('k') is currently modifying the global state of the colormap, which surprises me. Is there a reason for not always attaching a copy of the color map to the image object? I assumed that the following code would just affect the particular image color map, not the installed map:

    im = ax.pcolormesh(x, y, data, **kwargs)
    im.cmap.set_bad('k', 1.0)

It seems to me that, whatever the underlying design, this should just work.

Well, that is another wrinkle, I guess. Lets suppose I am good and make a copy of cmap before hand...

import matplotlib.cm as cm
import copy 

local_cmap = copy.copy(cm.RdBu_r)
im = ax.pcolormesh(x, y, data, cmap=local_cmap)
local_cmap.set_bad('k', 'organge')

Do I expect the set_bad to change the artist? I agree that im.cmap.set_bad should work regardless, but the question is if the artist creation should copy the colormap or not. You can see how folks are going to argue either direction.

I think that shows why the AxesImage instance should always store its own local copy of its cmap. If I came to this issue without preconceptions, I would assume that im.cmap would be unaffected by the subsequent call to local_cmap.set_bad. In my view, there should be no reason to believe that local_cmap still affects im after the call to pcolormesh is complete. Any changes should be made to im.cmap if you want to affect the existing plot.

I am guessing that the origin of this controversy is that, if in the past people have assumed that your code will change im.cmap, this will cause backward-compatibility issues for users. I am not really qualified to judge, but this still seems like a design flaw to me, and Matplotlib should make im.cmap a local copy, not make all cmaps immutable. However, there may be other subtleties that I don't fully understand.

@rayosborn

@rayosborn unless I misunderstood something, I think what we are trying to do is in line with your general ideas.

We have the design flaw that by default all images / colormaps you use are references to the builtin colormap objects. Thus any changes like set_bad() on them will modify the global state. This is bad practice we want to end.

To prevent users from modifying the global colormaps, there are two ways

a) Make the colormaps immutable. While this could be conceptually attractive, it's a larger API change, that would need strong justification. We're not following this path for now.
b) Don't give access to the global colormaps by always returning a copy when trying to access the colormaps.

We're following path b) as laid out in #16991.

Unfortunately, there are vailid use-cases that may have relied on modifcation of the global state, e.g.

plt.cm.viridis().set_bad('k')
plt.imshow()

To not break user code without prior notification, we needed the warning you've been seing.

Your example #19609 (comment) will continue to work. We only cannot distinguish the cases and therefore are erring on the side of warning too much than too little. The change to copy should have been implemented for 3.4, but we missed that and the RC is out, so we will likely extend the period one release and change to copy on 3.5. The warning will stay in-place for so long.

The copy variant is developed in #18503. Apparently, that has been re-milestoned to 3.5 anyway.

@jklymak

I may have misunderstood @jklymak's comment, but it seemed to suggest that there is still a debate about the issue.

So with v3.5, can I assume that any changes to im.cmap have no effect on cmap_d['viridis']? So., after

    from matplotlib.cm import cmap_d
    im = ax.pcolormesh(x, y, data, cmap=cmap_d['viridis'])
    im.cmap.set_bad('k', 1.0)

im contains its own copy of the cmap and cmap_d is not affected by the call to set_bad. That would be ideal.

However, I'm not sure I entirely agree though with the decision to issue these warnings. If I understand it, this design flaw has existed for years. To remove the warning message, I will have to modify my own package to make a copy of the color map for v3.4, but then remove this code when v3.5 is released. Since I want my package to be backwardly compatible, I will have to do a version check to modify the package behavior for the sake of a single Matplotlib version.

I suppose this only affects one or two package maintainers, so if you think the warning is required for interactive users, I can understand why you do it.

So with v3.5, can I assume, ...

Yes.

However, I'm not sure I entirely agree though with the decision to issue these warnings. If I understand it, this design flaw has existed for years. To remove the warning message, I will have to modify my own package to make a copy of the color map for v3.4, but then remove this code when v3.5 is released. Since I want my package to be backwardly compatible, I will have to do a version check to modify the package behavior for the sake of a single Matplotlib version.

The warning is already in 3.3. More importantly, your code is currently modifying the global 'viridis' colormap. If somebody uses viridis after your code, he will have the modified cmap. This is rather Matplotlibs fault than yours and as discussed here we'll fix that on our side, but that will take till 3.5 to not break other legitimate usages without prior notice. Not primarily to suppress the warning, but in the interest of your users, you should make a copy.

Note also that we'll be making the simple cmap_d private to prevent uncontrolled addition/overwriting of colormaps. The currently recommended way is register_cmap() for writing and get_cmap() for getting registered colormaps. We are working on a better interface for the future (#18503) but the two functions will remain available indefinitely for backward-compatibility.

Your code should look like this.

from matplotlib.cm import get_cmap
import copy
cmap = copy.copy(get_cmap('viridis'))
cmap.set_bad('k')
im = ax.pcolormesh(x, y, data, cmap=cmap)

This is will work for all existing and future versions of Matplotlib. I wouldn't even bother to do a version check for 3.5. The only disadvantage is that you do an unnecessary extra copy, but that's cheap enough.

Once you will only support matplotlib 3.5+ you'll probably be able to simplify this to something like

from matplotlib.cm import colormaps
im = ax.pcolormesh(x, y, data, cmap=colormaps['viridis'].with_extremes(bad='k'))

But we're still working on the details.

I suppose this only affects one or two package maintainers, so if you think the warning is required for interactive users, I can understand why you do it.

This also affects interactive users. Consider somebody using your code example or

cmap = get_cmap('viridis')
cmap.set_bad('k')
im = ax.pcolormesh(x, y, data, cmap=cmap)

in a jupyter notebook. This modifies the global 'viridis' cmap for the reset or the notebook and may very well not be intended.

Have we considered adding a kwarg to get_cmap('viridis', copy=True) to wrap the copy for folks?

AFAIK a copy kwarg has not been discussed.

However, I don't think it's the right thing here:

User-copy is only needed temporarily. If we introduce the kwarg, we either have to keep it around without effect forever, or we'll have to deprecate it later again and people who use it are forced to change their code again.
The advantage of copy.copy() is that it works with all past and future versions of matplotlib. While in future versions it's not needed, it's cheap enough so that people could leave it in their code if they don't want to adapt their code again or need backward compatibility with matplotlib < 3.5.
The warning is already part of matplotlib 3.3. I guess that a significant amount of people who care and do want to make a copy have already done so using copy.copy().

@timhoffm

@timhoffm, thanks for the detailed clarification. I will rewrite the NeXpy code to create copies where necessary as you suggest, at least until v3.5 is old enough to become a dependency.

Could I ask one quick question since we are on the subject? If cmap_d is going to be made private, how do I get a list of registered color maps? I need to know the name of a colormap before calling get_cmap.

@timhoffm

@timhoffm Asking folks to import copy and use copy.copy(cmap) to suppress a warning is a PITA. I think we should add a copy method to Colormap, or a copy kwarg to get_cmap. I suppose now that I think about it, a copy method on Colorbar is pretty trivial, but helpful in other situations. Then the old warning could be put back which is much nicer cmap = mpl.cm.get_cmap({cmap.name}).copy().

User-copy is only needed temporarily.

It would be nice to see the full proposal for how that will work and what the final API will be.

I don't think the desirability of removing global state is particularly up for debate, but how the colormap scope is handled below global is still not clear to me.

If cmap_d is going to be made private, how do I get a list of registered color maps?

I think the idea is to provide a Registry class that will presumably allow a view of the keys... #18503

See #19663 for what I think is a better temporary state of affairs.

User-copy is only needed temporarily.

It would be nice to see the full proposal for how that will work and what the final API will be.

We have not yet decided on the final API, but the basic plan is laid out at the top of #16991, with the exception that we have semi-intentionally pushed the changes mentioned for 3.4 to 3.5.

Essentially, we'll never give access to the global colormap directly but will always return a copy. Then, the user can do what ever she wants with that.

Whether or not we make colormaps immutable on top of that later is basically an independent discussion. Conceptually, immutablility would be nice - you can anyway only modify over/under/bad, the regular [0, 1]->color mapping cannot be changed via public API. However, I feel that deprecating set_over/under/bad would break too much user code. Without having made up my mind completely, I guess we could encourage people to treat colormaps as immutable and only use with_extremes(), but leave the set* methods in place for backward compatibility.

I find #16991 a bit too ambiguous. What is meant by "still return global"? From get_cmap? From cm.viridis?

So far it seems that we will allow colormaps to remain constant in user scope and below, so that regardless of where cmap comes from:

im1 = ax.pcolormesh(x, y, data, cmap=cmap)
im2 = ax.pcolormesh(x, y, data2, cmap=cmap)
assert im1.cmap == im2.cmap

will pass, and modifications to cmap will affect both im1 and im2. This is currently the case, and I guess I agree we shouldn't change it.

The plan I believe, is to make get_cmap() always return a copy of the colormap? So

im1 = ax.pcolormesh(x, y, data, cmap=cmap)
im2 = ax.pcolormesh(x, y, data2, cmap=cm.get_cmap(cmap))
assert im1.cmap == im2.cmap

will fail. That seems right to me.

However, I wonder if a lighter touch would be to keep cm.RdBu public and mutable, as it is now, and if the user wants the original to call get_cmap('RdBu_r'), with the string as argument. So we would have a private master list, and public mutable list.

I know that is not quite as protective as just not allowing the global to be modified, but it accomplishes a few things - it provides an easy way to make copies, which is what we want the user to do, and it makes it possible to get a safe version of the colormap.

So:

cmap1 = cm.RdBu                             # global RdBu colormap, in its current state
cmap1.set_over('green')                   # works
cmap1.get_over() == 'green'            # returns True.

cmap2 = cm.get_cmap(cm.RdBu)  # copy of cm.RdBu in its current state

cmap2.get_over() == 'green'           # returns True.

cmap = cm.get_cmap('RdBu')        # copy of _cmapd['RdBu'] registered state of colormap
cmap.get_over() == 'green'             # returns False

However, I wonder if a lighter touch would be to keep cm.RdBu public and mutable

I'm not sure about that. I've considered cm.RdBu equivalent to get_cmap('RdBu'). Having two mechanisms potentially return different results for the same name of the colormap is a source of confusion.

But the case of cm.[mapname] is not easy to solve. It is a global entity and changing that can have surprising effects on later code. OTOH being able to change it like

cmap1 = cm.RdBu
cmap1.set_over('green')

seems reasonable. We could make cm.RdBu a module property and always return a copy, but then again it looks like a variable and

cm.RdBu.set_over('green')
cm.RdBu.get_over() == 'green'   # fail

is worst of all. I could imagine that the least offensive way forward is to have an internal flag Colormap._is_immutable, which is true for the cm.[mapname] colormaps, but False for all copied maps. So

>>> cm.RdBu.set_over('green')
RuntimeError: cm.RdBu is a builtin colormap and cannot be modified. Please use cm.RdBu.copy()

Do I expect the set_bad to change the artist?

If we make a copy of the cmap on set or init then we make it impossible to intentionally couple artists where as if we keep a reference it is possible to have coupled or uncoupled at the users choice.

I think this is all covered by the plan in #20853, so no need to have a second issue tracking that.

Navigation Menu

Repository navigation

DeprecationWarning when changing color maps #19609

Proposed fix

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

DeprecationWarning when changing color maps #19609

Description

Proposed fix

Activity

jklymak commented on Mar 1, 2021

rayosborn commented on Mar 1, 2021

jklymak commented on Mar 1, 2021

rayosborn commented on Mar 1, 2021

timhoffm commented on Mar 5, 2021

rayosborn commented on Mar 6, 2021

timhoffm commented on Mar 6, 2021

jklymak commented on Mar 7, 2021

timhoffm commented on Mar 7, 2021

rayosborn commented on Mar 7, 2021

jklymak commented on Mar 7, 2021

jklymak commented on Mar 7, 2021

timhoffm commented on Mar 7, 2021

jklymak commented on Mar 7, 2021

timhoffm commented on Mar 8, 2021

tacaswell commented on Mar 24, 2021

QuLogic commented on Aug 24, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions