ENH/API: improvements to register_cmap #15127
Conversation
62403dc to
f2f5ca2
Compare
|
The general idea seems that matplotlib encourages a lot of third party packages to spawn in the wild, each with one or more colormaps, and each registering their colormaps via the |
|
Perhaps we should just not use string names and instead use normal qualified python names ( edit to clarify: If you're going to need an import to trigger the registration, you already have the module imported, so it's just as good to access an attribute from that module; because module names are unique (in a flat namespace), this handles the problem of name collision for us. |
|
As a meta comment, I mess w colormaps all the time and don’t ever bother registering them. But maybe that’s because I’ve never bothered to set up my own startup package. Agree w @ImportanceOfBeingErnest that being able to override colormap names should be allowed. If I want to replace jet with turbo and call it jet, why not? Overall, what is the end-user interface for colormaps? We really could use documentation on this. Are we all supposed to have our own pip installable packages myplottingsettings ? This feeds into the python matplotlibrc question |
|
In order to not be misunderstood here: I did not say one should allow for matplotlib's colormaps to be overridden. In fact a protection layer like the one proposed in this PR makes perfect sense. My comment above was about not protecting additional/external colormaps; or at least find a solution for the case that several packages try to register the same colormap under the same name. |
791c99a to
312d638
Compare
|
Flake8:
|
641c7c8 to
1b9e6a6
Compare
348fbbb to
0d2b6b2
Compare
|
Moved to new doc scheme and took @dstansby 's completion of the sentence. |
0d2b6b2 to
879522c
Compare
There was a problem hiding this comment.
Overall this is an improvement:
- Protecting builtin colormaps is our original interest.
- Warning on overwriting colormaps is also reasonable as one might otherwise have surprising effects.
- Unregistering is a way of silencing the above warning. There may be use cases, but it's not without problems because the same name may be used by multiple packages.
- I've proposed a note to at least make users aware of the issues.
- Name clashes could have happend before and that problem is not solved by unregister.
Still it's good to have an official function for that.
Not addressed by this PR:
- Strategies of name clashes and their handling.
879522c to
0b04abd
Compare
|
Where do we stand on this in light of #16943 and other discussions in the meeting? |
|
This has three approvals - just needs a rebase? |
|
If one of the original reviewers had a minute to re-review, I think that would be adequate to merge. |
|
Sorry, I'll explicitly ping because I'm not sure that re-requests show up in folks' feeds: @dopplershift @timhoffm @dstansby Thanks! |
Otherwise we will hand a view back to the user and it will change under them if `set_under` (e.g.) is called.
This was deprecated in 3.3 via matplotlib#15875 Enforce type of cmap as Colormap because we may not know that the object registered is of the wrong type until well after it was registered, defensively check here. We could do a duck-type check, but this is much simpler and we can cross the bridge of writing a duck-type checking function when someone has a use case for registering compatible-but-not-derived-from-Colormap instances.
3790fd7 to
4089b2c
Compare
|
ok, fixed and squashed down to 3 non-embarrassing commits (I was apparently working a bit longer than my brain was useful last night...). |
4089b2c to
86750a2
Compare
- raise if you try to over write a default colormap - warn if you try to over write a user-added colormap - add method to de-register a color map - add escape hatch to force re-registering builtin colormaps Co-Authored-By: David Stansby <dstansby@gmail.com>
86750a2 to
1a9dacf
Compare
|
Hi all, I recently got bitten by this change (my fault for not keeping my CI images up to date with the latest matplotlib). I am curious about the use of I maintain a MRI image viewer which comes with a collection of built in colour maps that, for historical reasons, use the same names as some built-in matplotlib colour maps, and am wondering about the best way to tackle this change in matplotlib. Thanks! |
|
@pauldmccarthy At this point I think you can rely on it being there, but re-affirm the note in the docstring "please only use this if you are sure you need it"! |
|
@tacaswell Yes, that particular note is why I was asking :) I'd thought of working around this by "salting" my own colour maps with a unique prefix, but I maintain my own colourmap registry (just containing the colour maps specific to my application) alongside |
|
That makes sense in the context of an application. "historical reasons" is something I have great personal sympathy with :) |
protect re-defining built in color maps
warn on re-defining user-defined color maps
add de-register function
Has Pytest style unit tests
Code is Flake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way