Skip to content
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

Colorspace conversions are wrong for anything other than uint8 #5979

Closed
tbenst opened this issue Oct 21, 2021 · 3 comments
Closed

Colorspace conversions are wrong for anything other than uint8 #5979

tbenst opened this issue Oct 21, 2021 · 3 comments
Labels
🫂 Support User help and QA

Comments

@tbenst
Copy link

tbenst commented Oct 21, 2021

Description

I've found what appears to be a severe bug, where colorspace conversions are completely wrong for all dtypes other than uint8.

Way to reproduce

import skimage.data
from skimage.color import rgb2luv
astronaut_uint8 = skimage.data.astronaut()
astronaut_float32 = astronaut_uint8.astype(np.float32)
astronaut_uint16 = astronaut_uint8.astype(np.uint16)
astronaut_uint8.max() # 255
astronaut_float32.max() # 255.0
rgb2luv(astronaut_uint8)[:,:,0].max() # 100.0
rgb2luv(astronaut_float32)[:,:,0].max() # 9341.576
rgb2luv(astronaut_uint16)[:,:,0].max() # 0.272

Note that max(L)=100 by definition, so 9341 is clearly wrong, as is 0.272.

Version information

3.7.3 (default, Mar 27 2019, 22:11:17) 
[GCC 7.3.0]
Linux-4.15.0-156-generic-x86_64-with-debian-buster-sid
scikit-image version: 0.18.1
numpy version: 1.20.3
@rfezzani
Copy link
Member

Hello @tbenst, and thank you for your interest in skimage. As the User Guide stipulates:

You should never use astype on an image, because it violates these assumptions about the dtype range...

🙂

>>> import skimage
>>> astronaut_uint8 = skimage.data.astronaut()
>>> astronaut_float32 = skimage.img_as_float32(astronaut_uint8)
>>> astronaut_uint16 = skimage.img_as_uint(astronaut_uint8)
>>> astronaut_uint8.max()
255
>>> astronaut_uint16.max()
65535
>>> astronaut_float32.max()
1.0
>>> rgb2luv(astronaut_uint8)[:,:,0].max()
100.0
>>> rgb2luv(astronaut_float32)[:,:,0].max()
100.0
>>> rgb2luv(astronaut_uint16)[:,:,0].max()
100.0

@rfezzani rfezzani added the 🫂 Support User help and QA label Oct 22, 2021
@rfezzani
Copy link
Member

@tbenst, if you don't mind I will convert this issue to conversation.

@scikit-image scikit-image locked and limited conversation to collaborators Oct 23, 2021
@mkcor mkcor closed this as completed Oct 23, 2021
@scikit-image scikit-image unlocked this conversation Apr 8, 2022
@grlee77
Copy link
Contributor

grlee77 commented Apr 8, 2022

prior comments from the deleted Discussion

@mkcor wrote

@tbenst you may also be interested in #5973

@hmaarrfk wrote

I guess the funny thing is that we are planning to move to the suggested behavior in skimage 1.0 (or is it 2.0 now)

@tbenst wrote

Ok, thank you very kindly for the explanation! That makes sense and helps a lot.

I do find the behavior odd / not intuitive, particularly as the function supports invalid inputs. In my actual code, I didn’t call astype but rather have float 0-255 after some simple numpy operations on UIn8 that promote to float. I personally think it’s not very pythonic for operations on numpy arrays to behave so differently depending on numerical precision, especially since (silent) type promotion happens all the time in numpy.

Perhaps the approach taken by Images.jl / Colors.jl would be useful here? It seems like introducing a type would be useful here, as the scikit-image isn’t really interpreting numpy arrays in the traditional way—and for good reasons, as images have different needs! The nice thing about the normalized 0-1 fixed point representation, is it converts to 0-1 float quite naturally. Also, the image float type constructor provides protection from passing invalid values. In Python that could be a type wrapper around a numpy array for minimal performance loss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🫂 Support User help and QA
Projects
None yet
Development

No branches or pull requests

4 participants