Skip to content

Don't fail on equal-but-differently-named cmaps in qt figureoptions. #29148

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.

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 1 commit into from
Nov 23, 2024

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 15, 2024

Currently, opening the Qt figureoptions UI for an image whose cmap is not registered in the colormap registry, has a name not matching any registry entry, but is actually equal (==, i.e. has the same LUT and colorbar-extension attributes) to a registry entry, leads to an error. A typical example would be

import cmap, pylab as p  # third-party
p.imshow([[0, 1]], cmap=cmap.Colormap("bids:magma").to_mpl())

and opening the qt figure options; this leads to the error "index 'bids:magma' is invalid ...".

Note that if the cmap is different from any registered cmap then we already add it to the UI combobox (try e.g. cmap=cmap.Colormap("imagej:fire")); the only problem was if it was equal to a registered cmap (this arises because when the code was originally written, cmap instance equality was by identity, not by comparing LUTs, so the cmap not in cm._colormaps.values() check behaved differently).

Fix that by checking whether the colormap name is registered. The behavior is still ill-defined in the opposite (theoretical) case of an unregistered cmap different from any registered cmap but with a matching name, but I'd argue that case is more pathological.

Test by running the above code and opening the qt figureoptions.

(aka how some code I wrote 9 years ago (#5469, a bit scary it's been that long) got broken by some code written 5.5 years later (#20227), and how to fix that with a one-line patch with 20 lines of explanation. I guess that's technically a regression :-))

PR summary

PR checklist

Currently, opening the Qt figureoptions UI for an image whose cmap is
not registered in the colormap registry, has a name not matching any
registry entry, but is actually equal (`==`, i.e. has the same LUT and
colorbar-extension attributes) to a registry entry, leads to an error.
A typical example would be
```
import cmap, pylab as p  # third-party
p.imshow([[0, 1]], cmap=cmap.Colormap("bids:magma").to_mpl())
```
and opening the qt figure options; this leads to the error "index
'bids:magma' is invalid ...".

Note that if the cmap is different from any registered
cmap then we already add it to the UI combobox (try e.g.
`cmap=cmap.Colormap("imagej:fire")`); the only problem was if it was
equal to a registered cmap (this arises because when the code was
originally written, cmap instance equality was by identity, not by
comparing LUTs, so the `cmap not in cm._colormaps.values()` check
behaved differently).

Fix that by checking whether the colormap *name* is registered.  The
behavior is still ill-defined in the opposite (theoretical) case of an
unregistered cmap different from any registered cmap but with a matching
name, but I'd argue that case is more pathological.

Test by running the above code and opening the qt figureoptions.
@QuLogic QuLogic added this to the v3.9.3 milestone Nov 23, 2024
@QuLogic QuLogic merged commit b1d2ecd into matplotlib:main Nov 23, 2024
44 of 46 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Nov 23, 2024
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Nov 23, 2024
@anntzer anntzer deleted the eqcm branch November 23, 2024 08:51
timhoffm added a commit that referenced this pull request Nov 24, 2024
…148-on-v3.9.x

Backport PR #29148 on branch v3.9.x (Don't fail on equal-but-differently-named cmaps in qt figureoptions.)
ksunden added a commit that referenced this pull request Dec 4, 2024
…148-on-v3.10.x

Backport PR #29148 on branch v3.10.x (Don't fail on equal-but-differently-named cmaps in qt figureoptions.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants