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.

Sign up for GitHub

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

Jump to bottom

MNT: improve image array argument checking in to_rgba. Closes #2499. #6122

Merged
merged 2 commits into from Mar 7, 2016
Merged

MNT: improve image array argument checking in to_rgba. Closes #2499. #6122

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8433eec
BUG Raise exception for invalid input
luispedro Oct 6, 2013
c2f91c5
MNT: improve image array argument checking in to_rgba. Closes #2499.
efiring Mar 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 16 additions & 9 deletions lib/matplotlib/cm.py
View file
Open in desktop
Expand Up @@ -221,6 +221,9 @@ def to_rgba(self, x, alpha=None, bytes=False, norm=True):
If *x* is an ndarray with 3 dimensions,
and the last dimension is either 3 or 4, then it will be
treated as an rgb or rgba array, and no mapping will be done.
The array can be uint8, or it can be floating point with
values in the 0-1 range; otherwise a ValueError will be raised.
If it is a masked array, the mask will be ignored.
If the last dimension is 3, the *alpha* kwarg (defaulting to 1)
will be used to fill in the transparency. If the last dimension
is 4, the *alpha* kwarg is ignored; it does not
Expand All @@ -232,12 +235,8 @@ def to_rgba(self, x, alpha=None, bytes=False, norm=True):
the returned rgba array will be uint8 in the 0 to 255 range.

If norm is False, no normalization of the input data is
performed, and it is assumed to already be in the range (0-1).
performed, and it is assumed to be in the range (0-1).

Note: this method assumes the input is well-behaved; it does
not check for anomalies such as *x* being a masked rgba
array, or being an integer type other than uint8, or being
a floating point rgba array with values outside the 0-1 range.
"""
# First check for special case, image input:
try:
Expand All @@ -255,10 +254,18 @@ def to_rgba(self, x, alpha=None, bytes=False, norm=True):
xx = x
else:
raise ValueError("third dimension must be 3 or 4")
if bytes and xx.dtype != np.uint8:
xx = (xx * 255).astype(np.uint8)
if not bytes and xx.dtype == np.uint8:
xx = xx.astype(float) / 255
if xx.dtype.kind == 'f':
if xx.max() > 1 or xx.min() < 0:
Copy link
Member

Choose a reason for hiding this comment

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

Kind of an expensive check for something that's on the mainline for displaying images. What happens if we remove this check?

Copy link
Member Author

Choose a reason for hiding this comment

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

How does this interact with your image refactor? I think that if this check is executed only on external input of 3-D RGBA arrays, then it is OK; that is really its purpose here. But if it is executed on internally-generated arrays in the normal course of events, then it needs to be bypassed. In fact, if that is happening, perhaps it is an entire call to the to_rgba method that needs to be bypassed, and the check can stay in place.

Copy link
Member

Choose a reason for hiding this comment

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

That's a good point. I do think it will only get run on externally-provided images -- but that does include images loaded using imread. I still wonder if it's too expensive even in that case though.

Copy link
Member

Choose a reason for hiding this comment

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

At least post-image-refactor these images are at "screen resolution" and not at data resolution which could be larger.

Copy link
Member

Choose a reason for hiding this comment

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

sorry for being trigger happy on this.

Copy link
Member

Choose a reason for hiding this comment

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

Having poked at it, for image I think it's impossible for this case to even happen -- the data is normalized (in any event) before it gets here. (However, I suspect that's not the case for other places to_rgba is used, like scatter).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think uses like scatter are irrelevant because the checking in question is invoked only for MxNx3 and MxNx4 inputs, which are assumed to be RGB or RGBA images.
As for "impossible": to_rgba is not private, so it is entirely possible for a user to generate a 3-D float array that would fail the check. If such an array is fed to imshow with interpolation 'none', will it be run through to_rgba? Certainly it will not be normalized.
The options are to remove the check on the grounds that it is usually unnecessary (we have managed without it for a long time), to leave it in and accept the performance penalty, or to find a way to distinguish between cases where it might be useful, if such cases exist, and cases where we are sure it is pointless. The last probably would be the best if it can be accomplished with minimal added complexity.

Copy link
Member

Choose a reason for hiding this comment

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

As a middle road, I think we can safely assume if norm=False is passed in that it's already normalized and we don't need to do this check. That will catch most of the cases where the check would be redundant.

raise ValueError("Floating point image RGB values "
"must be in the 0..1 range.")
if bytes:
xx = (xx * 255).astype(np.uint8)
elif xx.dtype == np.uint8:
if not bytes:
xx = xx.astype(float) / 255
else:
raise ValueError("Image RGB array must be uint8 or "
"floating point; found %s" % xx.dtype)
return xx
except AttributeError:
# e.g., x is not an ndarray; so try mapping it
Expand Down