MNT: Re-add matplotlib.cm.get_cmap #28355
Conversation
There was a problem hiding this comment.
I think mypy is complaining that it isn't public here.
This was showing up previously:
#28349 (comment)
So I agree it is likely people jumping more than 2 versions at a time.
This seems reasonable to extend the deprecation, but I still think it is a good idea to remove this function and push people towards what we want them to use, the colormaps registry. Users can still use plt.get_cmap instead of matplotlib.cm.get_cmap if they want a function call instead of dict lookup style access.
|
Just for reference, another use of get_cmap I've found in the wild: https://github.com/yt-project/yt_idv/blob/219338c989a1b67361893744385ddb3d543c8083/yt_idv/opengl_support.py#L224 I would be +0 for keeping that function around (especially as it's staying in pyplot!). Perhaps it could be discouraged in the docs, but it's also a bit weird because that'll just push people to use the pyplot version... whereas we usually don't want to push users in that direction (even though this case is of course quite different from the usual issues with pyplot). |
That is actually the edit: which makes me wonder if this function could be simplified to return |
|
We had at least one other case (which I am completely failing to find the issue for) where downstream never saw the warnings because they were running in a configuration where they were silent. |
The the old The only difference is that the exception for unexpected types is explicitly handled in |
|
FWIW the version in yt_idv was explicitly checking for ValueError, but at least in this case it is trivial to write code that works in all cases (one just needs to catch both ValueError and KeyError). |
|
Discussion on the call went the way of merging this; is there something left to move this out of draft? |
|
Is there a consensus on temporarily vs permanent reactivation? I‘m now +0 on permanent but discouraged. I suppose I may have underestimated people using semi-OT: It may be worth expanding on discouraging. I.e. keeping API but steering people away. Including optionally issuing runtime warnings. This can move the usage patterns over time. -> better user code for most, no forced changes. A possible later deprecation will be less disruptive. |
|
Consensus was "temporary" |
This was removed in 3.9, but apparently caused more user trouble than expected. Re-added for 3.9.1 and extended the deprecation period for two additional minor releases.
…355-on-v3.9.x Backport PR #28355 on branch v3.9.x (MNT: Re-add matplotlib.cm.get_cmap)
Addresses #28349 (comment)
This was removed in 3.9, but apparently caused more user trouble than expected. Re-added for 3.9.1 and extended the deprecation period for two additional minor releases.
I did not find a technical reason why people may not see it. My wild guess is that this was quite commonly used and there are enough cases where people jump more than 2 minor versions. Hopefully extending for 2 additional minor releases will be good enough. - I question whether this was too common and should not have been removed at all, but now that we've started IMHO we should eventually remove.