Skip to content

Clip RGB data to valid range for imshow #10220

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

Merged
merged 1 commit into from
Feb 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/api/api_changes/2018-01-26-ZHD.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
`Axes.imshow` clips RGB values to the valid range
-------------------------------------------------

When `Axes.imshow` is passed an RGB or RGBA value with out-of-range
values, it now logs a warning and clips them to the valid range.
The old behaviour, wrapping back in to the range, often hid outliers
and made interpreting RGB images unreliable.
1 change: 1 addition & 0 deletions doc/users/credits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ Yu Feng,
Yunfei Yang,
Yuri D'Elia,
Yuval Langer,
Zac Hatfield-Dodds,
Zach Pincus,
Zair Mubashar,
alex,
Expand Down
7 changes: 7 additions & 0 deletions doc/users/next_whats_new/2018_01_26_imshow-rgb-clipping.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
`Axes.imshow` clips RGB values to the valid range
-------------------------------------------------

When `Axes.imshow` is passed an RGB or RGBA value with out-of-range
values, it now logs a warning and clips them to the valid range.
The old behaviour, wrapping back in to the range, often hid outliers
and made interpreting RGB images unreliable.
15 changes: 10 additions & 5 deletions lib/matplotlib/axes/_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -5323,10 +5323,14 @@ def imshow(self, X, cmap=None, norm=None, aspect=None,
- MxNx3 -- RGB (float or uint8)
- MxNx4 -- RGBA (float or uint8)

The value for each component of MxNx3 and MxNx4 float arrays
should be in the range 0.0 to 1.0. MxN arrays are mapped
to colors based on the `norm` (mapping scalar to scalar)
and the `cmap` (mapping the normed scalar to a color).
MxN arrays are mapped to colors based on the `norm` (mapping
scalar to scalar) and the `cmap` (mapping the normed scalar to
a color).

Elements of RGB and RGBA arrays represent pixels of an MxN image.
All values should be in the range [0 .. 1] for floats or
[0 .. 255] for integers. Out-of-range values will be clipped to
these bounds.

cmap : `~matplotlib.colors.Colormap`, optional, default: None
If None, default to rc `image.cmap` value. `cmap` is ignored
Expand Down Expand Up @@ -5368,7 +5372,8 @@ def imshow(self, X, cmap=None, norm=None, aspect=None,
settings for `vmin` and `vmax` will be ignored.

alpha : scalar, optional, default: None
The alpha blending value, between 0 (transparent) and 1 (opaque)
The alpha blending value, between 0 (transparent) and 1 (opaque).
The ``alpha`` argument is ignored for RGBA input data.

origin : ['upper' | 'lower'], optional, default: None
Place the [0,0] index of the array in the upper left or lower left
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/cm.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ def to_rgba(self, x, alpha=None, bytes=False, norm=True):
xx = (xx * 255).astype(np.uint8)
elif xx.dtype == np.uint8:
if not bytes:
xx = xx.astype(float) / 255
xx = xx.astype(np.float32) / 255
else:
raise ValueError("Image RGB array must be uint8 or "
"floating point; found %s" % xx.dtype)
Expand Down
20 changes: 20 additions & 0 deletions lib/matplotlib/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from math import ceil
import os
import logging

import numpy as np

Expand All @@ -34,6 +35,8 @@
from matplotlib.transforms import (Affine2D, BboxBase, Bbox, BboxTransform,
IdentityTransform, TransformedBbox)

_log = logging.getLogger(__name__)

# map interpolation strings to module constants
_interpd_ = {
'none': _image.NEAREST, # fall back to nearest when not supported
Expand Down Expand Up @@ -623,6 +626,23 @@ def set_data(self, A):
or self._A.ndim == 3 and self._A.shape[-1] in [3, 4]):
raise TypeError("Invalid dimensions for image data")

if self._A.ndim == 3:
# If the input data has values outside the valid range (after
# normalisation), we issue a warning and then clip X to the bounds
# - otherwise casting wraps extreme values, hiding outliers and
# making reliable interpretation impossible.
high = 255 if np.issubdtype(self._A.dtype, np.integer) else 1
if self._A.min() < 0 or high < self._A.max():
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should pad this by an eps in case the 0-1 values came from some numerical computation, and we probably wouldn't want to spam the user with unnecessary warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to clip to exactly [0 .. 1], and for consistency I would prefer to always warn if clipping. If users feel spammed, they can either fix their data or filter the logging output.

Copy link
Member

Choose a reason for hiding this comment

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

I think the suggestion might have been to silently clip if (a-vmax)/(vmin - vmax) - 1 < 1e-10 or something that would just represent bit noise that would never affect the interpretation of the data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's the question - I just think the answer is "No, we shouldn't".

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree that we shouldn't.

_log.warning(
'Clipping input data to the valid range for imshow with '
'RGB data ([0..1] for floats or [0..255] for integers).'
)
self._A = np.clip(self._A, 0, high)
# Cast unsupported integer types to uint8
if self._A.dtype != np.uint8 and np.issubdtype(self._A.dtype,
np.integer):
self._A = self._A.astype(np.uint8)

self._imcache = None
self._rgbacache = None
self.stale = True
Expand Down
23 changes: 22 additions & 1 deletion lib/matplotlib/tests/test_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ def test_minimized_rasterized():
def test_load_from_url():
req = six.moves.urllib.request.urlopen(
"http://matplotlib.org/_static/logo_sidebar_horiz.png")
Z = plt.imread(req)
plt.imread(req)


@image_comparison(baseline_images=['log_scale_image'],
Expand Down Expand Up @@ -815,6 +815,27 @@ def test_imshow_no_warn_invalid():
assert len(warns) == 0


@pytest.mark.parametrize(
'dtype', [np.dtype(s) for s in 'u2 u4 i2 i4 i8 f4 f8'.split()])
def test_imshow_clips_rgb_to_valid_range(dtype):
arr = np.arange(300, dtype=dtype).reshape((10, 10, 3))
if dtype.kind != 'u':
arr -= 10
too_low = arr < 0
too_high = arr > 255
if dtype.kind == 'f':
arr = arr / 255
_, ax = plt.subplots()
out = ax.imshow(arr).get_array()
assert (out[too_low] == 0).all()
if dtype.kind == 'f':
assert (out[too_high] == 1).all()
assert out.dtype.kind == 'f'
else:
assert (out[too_high] == 255).all()
assert out.dtype == np.uint8


@image_comparison(baseline_images=['imshow_flatfield'],
remove_text=True, style='mpl20',
extensions=['png'])
Expand Down