Skip to content

Issues with colormap documentation and behavior #6080

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

Closed
2 tasks
jni opened this issue Mar 1, 2016 · 6 comments
Closed
2 tasks

Issues with colormap documentation and behavior #6080

jni opened this issue Mar 1, 2016 · 6 comments

Comments

@jni
Copy link
Contributor

jni commented Mar 1, 2016

Over at scikit-image we were trying to write a function that would transform a grayscale image into a colormapped image of shape im.shape + (4,), when @tacaswell pointed out that cmap(image) does this already, though this doesn't appear in the MPL docs. It turns out that this is a flaw in sphinx/numpydoc: __call__ is ignored. The method itself is in fact very well documented:

https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/colors.py#L525

So that's the first issue:

  • fix sphinx/numpydoc/config/documentation so that the above functionality is exposed in the documentation.

The second issue is that we would probably want to use the normalizing version of this, ScalarMappable.to_rgba:

https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/cm.py#L211

However, it's actually broken for nD cases, because a 3D image will unnecessarily cause a ValueError (here). So, the second (perhaps optional) issue:

  • fix ScalarMappable.to_rgba behavior so that 3D grayscale images (numpy arrays) are supported.

Happy to submit PRs for this but I'd like for some core devs to chime in regarding how best to deal with these... (In particular, I have zero experience with sphinx configuration.)

@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Jun 13, 2016
@tacaswell tacaswell modified the milestones: 2.0.1 (next bug fix release), 2.0 (style change major release) Jul 16, 2016
@QuLogic QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0.2 (next bug fix release) May 3, 2017
@tacaswell tacaswell modified the milestones: 2.1.1 (next bug fix release), 2.2 (next feature release) Oct 9, 2017
@QuLogic
Copy link
Member

QuLogic commented Jul 8, 2021

__call__ is now documented, probably thanks to #20081.

I'm not sure what you mean about 3D images. If you know it's greyscale, just pass one channel to the colour mapping. Matplotlib can't know this is what you want.

@jklymak
Copy link
Member

jklymak commented Jul 8, 2021

Yes, this has been discussed elsewhere as well - we don't want to guess if the image is grayscale or not. I'll close...

@jklymak jklymak closed this as completed Jul 8, 2021
@jni
Copy link
Contributor Author

jni commented Jul 21, 2021

we don't want to guess if the image is grayscale or not

Um, but you do, here:

if x.ndim == 3:
if x.shape[2] == 3:
if alpha is None:
alpha = 1
if x.dtype == np.uint8:
alpha = np.uint8(alpha * 255)
m, n = x.shape[:2]
xx = np.empty(shape=(m, n, 4), dtype=x.dtype)
xx[:, :, :3] = x
xx[:, :, 3] = alpha
elif x.shape[2] == 4:
xx = x
else:
raise ValueError("Third dimension must be 3 or 4")

If I use to_rgba on a 3D grayscale image (e.g. shape (100, 100, 100)), a ValueError is raised, because you are guessing that I'm giving you a 2D RGB(A) array.

@jklymak
Copy link
Member

jklymak commented Jul 21, 2021

That is correct. We don't accept 3-d arrays unless the third dimension has length 3 or 4, in which case we interpret as rgb(a). If we accepted other 3D arrays how would we know if a (100,100,3) array was rgb or three (100,100) greyscale arrays?

@jni
Copy link
Contributor Author

jni commented Jul 23, 2021

I just think the code is in a weird place where 1D, 2D, 4D, 5D, and 27D arrays work fine, but 3D arrays are special-cased in case the user happens to have passed in a 2D RGB(A) array.

To enable full nD support, you could add a guess_rgba=True kwarg that would keep the existing guessing, but that users could disable.

But anyway, this issue is from a long time ago, and I can see that there's backwards compatibility concerns, so 🤷. I wouldn't be using it in the near term, it's just the special casing that bugs me, as a frequent user of 3D grayscale images. =)

@jklymak
Copy link
Member

jklymak commented Jul 23, 2021

I see, you are correct that higher dim arrays are not explicitly forbidden in the code. If it would be useful to folks, I don't see a reason to not skip the check if guess_rgba=False.

This is really just a one-liner at this point though: rgba = cmap(norm(x), alpha=alpha)

@story645 story645 removed this from the future releases milestone Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants