Skip to content

ENH: Add manual value limits to OrthoSlicer3D/SpatialImages.orthoview #491

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

Closed
wants to merge 4 commits into from
Closed

ENH: Add manual value limits to OrthoSlicer3D/SpatialImages.orthoview #491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7178864
ENH: Add manual value limits to OrthoSlicer3D
effigies Oct 16, 2016
91d3000
DOC: Use explicit args in SpatialImage.orthoview
effigies Oct 19, 2016
148c074
ENH: Enable percentile value limits
effigies Oct 19, 2016
0cd1411
TEST Test percentiles
effigies Oct 19, 2016
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
15 changes: 12 additions & 3 deletions nibabel/spatialimages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,9 +588,18 @@ def __getitem__(self, idx):
"slicing image array data with `img.dataobj[slice]` or "
"`img.get_data()[slice]`")

def orthoview(self):
def orthoview(self, axes=None, vlim=None):
"""Plot the image using OrthoSlicer3D

Parameters
------------------
axes : tuple of mpl.Axes or None, optional
3 or 4 axes instances for the 3 slices plus volumes,
or None (default).
vlim : array-like or None, optional
Copy link
Member

Choose a reason for hiding this comment

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

Copy vlim docstring entry from OrthoSlicer3D?

Value limits to display image and time series. Can be None
(default) to derive limits from data.

Returns
-------
viewer : instance of OrthoSlicer3D
Expand All @@ -602,8 +611,8 @@ def orthoview(self):
consider using viewer.show() (equivalently plt.show()) to show
the figure.
"""
return OrthoSlicer3D(self.dataobj, self.affine,
title=self.get_filename())
return OrthoSlicer3D(self.dataobj, self.affine, axes=axes,
title=self.get_filename(), vlim=vlim)

def as_reoriented(self, ornt):
"""Apply an orientation change and return a new image
Expand Down
11 changes: 11 additions & 0 deletions nibabel/tests/test_spatialimages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from numpy.testing import assert_array_equal, assert_array_almost_equal

from .test_helpers import bytesio_round_trip
from .test_viewers import needs_mpl
from ..testing import (clear_and_catch_warnings, suppress_warnings,
memmap_after_ufunc)
from ..tmpdirs import InTemporaryDirectory
Expand Down Expand Up @@ -539,6 +540,16 @@ def test_slicer(self):
assert_array_equal(sliced_data, img.dataobj[sliceobj])
assert_array_equal(sliced_data, img.get_data()[sliceobj])

@needs_mpl
def test_orthoview(self):
# Assumes all possible images support int16
# See https://github.com/nipy/nibabel/issues/58
arr = np.arange(24, dtype=np.int16).reshape((2, 3, 4))
img = self.image_class(arr, None)
img.orthoview().close()
img.orthoview(vlim=(5, 10)).close()
img.orthoview(slicer=Ellipsis).close()

def test_api_deprecations(self):

class FakeImage(self.image_class):
Expand Down
19 changes: 18 additions & 1 deletion nibabel/tests/test_viewers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from ..testing import skipif
from numpy.testing import assert_array_equal, assert_equal

from nose.tools import assert_raises, assert_true
from nose.tools import assert_raises, assert_true, assert_false

# Need at least MPL 1.3 for viewer tests.
matplotlib, has_mpl, _ = optional_package('matplotlib', min_version='1.3')
Expand Down Expand Up @@ -68,6 +68,23 @@ def test_viewer():
v.close()
v._draw() # should be safe

# Manually set value limits
vlim = np.array([-20, 20])
v = OrthoSlicer3D(data, vlim=vlim)
assert_array_equal(v._clim, vlim)
for im in v._ims:
assert_array_equal(im.get_clim(), vlim)
assert_array_equal(v._axes[3].get_ylim(), vlim)
v.close()
v1 = OrthoSlicer3D(data)
v2 = OrthoSlicer3D(data, vlim=('1%', '99%'))
assert_array_equal(v1.clim, v2.clim)
v2.close()
v2 = OrthoSlicer3D(data, vlim=('2%', '98%'))
assert_false(np.array_equal(v1.clim, v2.clim))
v2.close()
v1.close()

# non-multi-volume
v = OrthoSlicer3D(data[:, :, :, 0])
v._on_scroll(nt('event', 'button inaxes key')('up', v._axes[0], 'shift'))
Expand Down
22 changes: 18 additions & 4 deletions nibabel/viewers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class OrthoSlicer3D(object):
"""
# Skip doctest above b/c not all systems have mpl installed

def __init__(self, data, affine=None, axes=None, title=None):
def __init__(self, data, affine=None, axes=None, title=None, vlim=None):
"""
Parameters
----------
Expand All @@ -61,6 +61,10 @@ def __init__(self, data, affine=None, axes=None, title=None):
title : str or None, optional
The title to display. Can be None (default) to display no
title.
vlim : array-like or None, optional
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of also allowing strings like 10% 90% as input, meaning percentile? Then the default could be vlim=('1%', '99%').

Copy link
Member Author

Choose a reason for hiding this comment

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

That works for clim, but ylim's default is a bit more complicated.

Copy link
Member

Choose a reason for hiding this comment

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

Aha. I see there's a heuristic for the colorbar around line 205 - is that what you mean?

How about throwing that away and just using the data limits as set by vlim? As in something like:

if vlim is None:
    vlim = ('1%', '99%')

etc.

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 believe the purpose of that heuristic is so that the time-series is guaranteed to fit in the pane, with a slight buffer on top and bottom for visibility. If we set ylim(1%, 99%), then some values won't fit in the pane, by default.

Copy link
Member

Choose a reason for hiding this comment

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

Won't that be true for any not-default limits that we set? I mean, don't we need to apply the heuristic for any limits?

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'm not sure I fully understand this comment.

The three use-cases I see, myself, are (1) exploration, (2) comparison, (3) figure-generation. I think the existing default behavior is good for (1). I think the proposed changes are good for (2)+(3), where consistency/predictability is more valuable than smart estimates.

The problem I see in your proposed change is that you're changing the default behavior to something a little less friendly by setting ylim == clim even in the default case.

Apologies if I'm not making much sense.

Copy link
Member

Choose a reason for hiding this comment

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

Sure - I agree that the ylim == clim thing is bad in the default case, the question is what to do in the not-default case.

For example - I guess that people will want to use vlim to get better contrast in the images, but they don't intend to change the time-series pane. Do you think that's right? In which case, should ylim be another input argument, that defaults to the original heuristic, regardless of vlim?

Copy link
Member

Choose a reason for hiding this comment

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

@effigies - any comments here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @matthew-brett. Sorry, busy times.

To clarify my usage when I was working on this: I would have multiple files with different ranges. By setting vlim, I would be able to directly compare them, both the images and the time series. So for my case, I definitely think keeping them the same was preferable.

However, if what we want is a separate ylim, we can do that. In which case, should we change vlim to clim to drive home that one is for images, one is for time series? I chose vlim initially specifically to abstract from y/c.

What do you think?

Value limits to display image and time series. Can be None
(default) to derive limits from data. Bounds can be of the
form ``'x%'`` to use the ``x`` percentile of the data.
"""
# Use these late imports of matplotlib so that we have some hope that
# the test functions are the first to set the matplotlib backend. The
Expand All @@ -79,6 +83,13 @@ def __init__(self, data, affine=None, axes=None, title=None):
affine = np.array(affine, float) if affine is not None else np.eye(4)
if affine.shape != (4, 4):
raise ValueError('affine must be a 4x4 matrix')

if vlim is not None:
percentiles = all(isinstance(lim, str) and lim[-1] == '%'
for lim in vlim)
if percentiles:
vlim = np.percentile(data, [float(lim[:-1]) for lim in vlim])

# determine our orientation
self._affine = affine
codes = axcodes2ornt(aff2axcodes(self._affine))
Expand All @@ -91,7 +102,7 @@ def __init__(self, data, affine=None, axes=None, title=None):
self._volume_dims = data.shape[3:]
self._current_vol_data = data[:, :, :, 0] if data.ndim > 3 else data
self._data = data
self._clim = np.percentile(data, (1., 99.))
self._clim = np.percentile(data, (1., 99.)) if vlim is None else vlim
del data

if axes is None: # make the axes
Expand Down Expand Up @@ -184,8 +195,11 @@ def __init__(self, data, affine=None, axes=None, title=None):
ax.set_xticks(np.unique(np.linspace(0, self.n_volumes - 1,
5).astype(int)))
ax.set_xlim(x[0], x[-1])
yl = [self._data.min(), self._data.max()]
yl = [l + s * np.diff(lims)[0] for l, s in zip(yl, [-1.01, 1.01])]
if vlim is None:
yl = [self._data.min(), self._data.max()]
yl = [l + s * np.diff(lims)[0] for l, s in zip(yl, [-1.01, 1.01])]
else:
yl = vlim
patch = mpl_patch.Rectangle([-0.5, yl[0]], 1., np.diff(yl)[0],
fill=True, facecolor=(0, 1, 0),
edgecolor=(0, 1, 0), alpha=0.25)
Expand Down