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

BUG Raise exception for invalid input #2632

Closed
wants to merge 2 commits into from

Conversation

luispedro
Copy link
Contributor

Arrays of non-uint8 types were assumed to be in 0..1 range.

When this was not true and integer values were used, only the low-order
bits were used, resulting in a mangled image. Now, an explicit exception
is raised.

Closes issue #2499

Arrays of non-uint8 types were assumed to be in 0..1 range.

When this was not true and integer values were used, only the low-order
bits were used, resulting in a mangled image. Now, an explicit exception
is raised.

Closes issue matplotlib#2499
@@ -248,6 +248,8 @@ def to_rgba(self, x, alpha=None, bytes=False):
xx = x
else:
raise ValueError("third dimension must be 3 or 4")
if xx.dtype != np.uint8 and xx.max() > 1:
raise ValueError("image must be either uint8 or in the 0..1 range")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failure is a pep8 violation on this line.

@tacaswell
Copy link
Member

Sorry, I didn't look at this change closely enough last night. The docstring on to_rgba says it does not to these checks, so the fix should probably go in image._AxesImageBase.set_data

@efiring
Copy link
Member

efiring commented Nov 30, 2013

I don't think we want to be doing this 0-1 check at all. What would make sense is to raise an exception if the input is integer type but not uint8, since uint8 is the only supported integer type. This check is fast to do, and entirely consistent with the logic of the method; it would require a change to the docstring to remove the statement that dtype is not checked. It would have to be clear that the integer-is-uint8 check is done only in the rgb or rgba case. In the mappable array case, any integer type is allowed, triggering direct indexing into the colormap LUT.

@luispedro
Copy link
Contributor Author

Right now matplotlib correctly displays binary images. As a user, I'd prefer if it kept that functionality.

@efiring
Copy link
Member

efiring commented Dec 1, 2013

@luispedro Would you elaborate, please, on your last comment? I don't understand what it means with respect to this PR, and my preceding recommendation. What do you mean by "displays binary images"? With what kinds of input array?

@luispedro
Copy link
Contributor Author

I meant boolean images. For example:

X,Y = np.meshgrid(np.arange(128), np.arange(128))
imshow((X**2 + Y**2) < 128**2)

I feel that at least uint8 and bool should be supported; otherwise it's a functionality loss.

@tacaswell tacaswell added this to the v1.5.x milestone Aug 17, 2014
@tacaswell tacaswell modified the milestones: proposed next point release, next point release Feb 19, 2015
@tacaswell tacaswell modified the milestones: next point release, Color overhaul Jul 17, 2015
@mdboom mdboom modified the milestones: Color overhaul, next major release (2.0) Oct 8, 2015
@tacaswell tacaswell closed this Feb 29, 2016
@tacaswell tacaswell reopened this Feb 29, 2016
@efiring
Copy link
Member

efiring commented Mar 7, 2016

Included in #6122.

@efiring efiring closed this Mar 7, 2016
@QuLogic QuLogic added this to the 2.1 (next point release) milestone Sep 28, 2016
@QuLogic QuLogic removed this from the 2.0 (style change major release) milestone Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants