-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: handle nans in RGBA input with ScalarMappables #27848
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sympathetic to the view that it is easier to push in a "nan" and expect nothing to be plotted rather than to be forced into using masked arrays by raising a ValueError if a nan appears. I can see both sides here though. This seems like a small amount of code, so nothing crazy complex to support this use-case.
lib/matplotlib/cm.py
Outdated
# If any of R, G, B, or A are masked for an entry, we set alpha to 0 | ||
xx[np.any(np.isnan(x), axis=2), 3] = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to move this up into only the floating point portion so we aren't checking for nan's on non-float data.
lib/matplotlib/cm.py
Outdated
else: | ||
raise ValueError("Third dimension must be 3 or 4") | ||
if xx.dtype.kind == 'f': | ||
if norm and (xx.max() > 1 or xx.min() < 0): | ||
raise ValueError("Floating point image RGB values " | ||
"must be in the 0..1 range.") | ||
# If any of R, G, B, or A is nan, set to 0 | ||
xx[np.any(np.isnan(x), axis=2), :] = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The casting for the bytes=True
case will error if there are nans in the array so we need to replace the nan with an arbitrary float. Setting the whole pixel to 0 seems the simplest way to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is too late; if xx = np.array([10, np.nan, 0.2])
, then the above check should fail, but it doesn't, because xx.max() == np.nan
, and np.nan > 1
is False.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I just move this above the range check, then we replace the pixel with zeros which will make the range check pass. So there will be no difference. Do we want to do something more complicated than set the whole pixel to zero?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That depends on if we want to ignore all of the pixel that is NaN, regardless of value or not. We can replace with np.nanmax
/np.nanmin
in that case. If not, then we need to do something more complicated.
But my point is more that the check is broken even if the NaN and the invalid value are not in the same pixel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But my point is more that the check is broken even if the NaN and the invalid value are not in the same pixel.
Oh yes, I get it now.
I think the question is in the case of RGBA images do we want:
On net I think I come down in favor of this PR over mine. |
Doc build fails because of warnings from #27658 |
I think a rebase on to main should fix the doc build. |
Update following discussion on call. The walrus is for @tacaswell :-) |
PR summary
Fixes #27301 by setting alpha to zero at points where any value is nan. This is consistent with the handling added for masked arrays at #26096.
Note this PR competes with #27303 which goes the other way and raises when nans are in the array.
PR checklist