From 9876563a43743a3734c269a8a922c586788defa2 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Sun, 16 Nov 2014 02:20:02 -0800 Subject: [PATCH 1/4] DOC: typo in comment --- nibabel/spatialimages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nibabel/spatialimages.py b/nibabel/spatialimages.py index 73104e4ca5..303f3ab40d 100644 --- a/nibabel/spatialimages.py +++ b/nibabel/spatialimages.py @@ -176,7 +176,7 @@ def from_header(klass, header=None): if header is None: return klass() # I can't do isinstance here because it is not necessarily true - # that a subclass has exactly the same interface as it's parent + # that a subclass has exactly the same interface as its parent # - for example Nifti1Images inherit from Analyze, but have # different field names if type(header) == klass: From 932f237a57f54914411290da2199cfefe03e6dcd Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Sun, 16 Nov 2014 02:25:27 -0800 Subject: [PATCH 2/4] NF: add in_memory property `in_memory` property is True if image has an array as its data object, or if a proxy image has been cached. --- nibabel/spatialimages.py | 7 +++++++ nibabel/tests/test_image_api.py | 13 ++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/nibabel/spatialimages.py b/nibabel/spatialimages.py index 303f3ab40d..67df60e435 100644 --- a/nibabel/spatialimages.py +++ b/nibabel/spatialimages.py @@ -464,6 +464,13 @@ def get_data(self): self._data_cache = np.asanyarray(self._dataobj) return self._data_cache + @property + def in_memory(self): + """ True when array data is in memory + """ + return (isinstance(self._dataobj, np.ndarray) + or self._data_cache is not None) + def uncache(self): """ Delete any cached read of data from proxied data diff --git a/nibabel/tests/test_image_api.py b/nibabel/tests/test_image_api.py index 1565d165a2..72f2653b79 100644 --- a/nibabel/tests/test_image_api.py +++ b/nibabel/tests/test_image_api.py @@ -17,6 +17,8 @@ array creation. If it does, this call empties that cache. Implement this as a no-op if ``get_data()`` does not cache. * ``img[something]`` generates an informative TypeError +* ``img.in_memory`` is True for an array image, and for a proxy image that is + cached, but False otherwise. """ from __future__ import division, print_function, absolute_import @@ -172,7 +174,11 @@ def validate_data(self, imaker, params): assert_false(isinstance(img.dataobj, np.ndarray)) proxy_data = np.asarray(img.dataobj) proxy_copy = proxy_data.copy() + # Not yet cached, proxy image: in_memory is False + assert_false(img.in_memory) data = img.get_data() + # Data now cached + assert_true(img.in_memory) assert_false(proxy_data is data) # changing array data does not change proxy data, or reloaded data data[:] = 42 @@ -182,9 +188,12 @@ def validate_data(self, imaker, params): assert_array_equal(img.get_data(), 42) # until we uncache img.uncache() + # Which unsets in_memory + assert_false(img.in_memory) assert_array_equal(img.get_data(), proxy_copy) else: # not proxy assert_true(isinstance(img.dataobj, np.ndarray)) + assert_true(img.in_memory) non_proxy_data = np.asarray(img.dataobj) data = img.get_data() assert_true(non_proxy_data is data) @@ -196,9 +205,11 @@ def validate_data(self, imaker, params): # Unache has no effect img.uncache() assert_array_equal(img.get_data(), 42) - # Read only + # dataobj is read only fake_data = np.zeros(img.shape).astype(img.get_data_dtype()) assert_raises(AttributeError, setattr, img, 'dataobj', fake_data) + # So is in_memory + assert_raises(AttributeError, setattr, img, 'in_memory', False) def validate_data_deprecated(self, imaker, params): # Check _data property still exists, but raises warning From a8e5b82282512b6ec0989a79033723d0607a33f8 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Mon, 24 Nov 2014 17:21:00 -0800 Subject: [PATCH 3/4] NF: add "caching" flag to ``get_data`` Caching flag can take values 'fill' and 'unchanged'. --- nibabel/spatialimages.py | 30 +++++++++++++++---- nibabel/tests/test_image_api.py | 51 ++++++++++++++++++++++++--------- 2 files changed, 62 insertions(+), 19 deletions(-) diff --git a/nibabel/spatialimages.py b/nibabel/spatialimages.py index 67df60e435..ace6fd0e48 100644 --- a/nibabel/spatialimages.py +++ b/nibabel/spatialimages.py @@ -448,21 +448,39 @@ def __str__(self): 'metadata:', '%s' % self._header)) - def get_data(self): + def get_data(self, caching='fill'): """ Return image data from image with any necessary scalng applied If the image data is a array proxy (data not yet read from disk) then - read the data, and store in an internal cache. Future calls to - ``get_data`` will return the cached copy. + the default behavior (`caching` == "fill") is to read the data, and + store in an internal cache. Future calls to ``get_data`` will return + the cached copy. + + Parameters + ---------- + caching : {'fill', 'unchanged'}, optional + This argument has no effect in the case where the image data is an + array, or the image data has already been cached. If the image data + is an array proxy, and the image data has not yet been cached, then + 'fill' (the default) will read the data from the array proxy, and + store in an internal cache, so that future calls to ``get_data`` + will return the cached copy. If 'unchanged' then leave the cache + unchanged; return the cached copy if it exists, if not, load the + data from disk and return that, but without filling the cache. Returns ------- data : array array of image data """ - if self._data_cache is None: - self._data_cache = np.asanyarray(self._dataobj) - return self._data_cache + if not caching in ('fill', 'unchanged'): + raise ValueError('caching value should be "fill" or "unchanged"') + if self._data_cache is not None: + return self._data_cache + data = np.asanyarray(self._dataobj) + if caching == 'fill': + self._data_cache = data + return data @property def in_memory(self): diff --git a/nibabel/tests/test_image_api.py b/nibabel/tests/test_image_api.py index 72f2653b79..fed9f85d5b 100644 --- a/nibabel/tests/test_image_api.py +++ b/nibabel/tests/test_image_api.py @@ -23,6 +23,7 @@ from __future__ import division, print_function, absolute_import import warnings +from functools import partial import numpy as np @@ -176,10 +177,21 @@ def validate_data(self, imaker, params): proxy_copy = proxy_data.copy() # Not yet cached, proxy image: in_memory is False assert_false(img.in_memory) + # Load with caching='unchanged' + data = img.get_data(caching='unchanged') + # Still not cached + assert_false(img.in_memory) + # Default load, does caching data = img.get_data() # Data now cached assert_true(img.in_memory) assert_false(proxy_data is data) + # Now caching='unchanged' does nothing, returns cached version + data_again = img.get_data(caching='unchanged') + assert_true(data is data_again) + # caching='fill' does nothing because the cache is already full + data_yet_again = img.get_data(caching='fill') + assert_true(data is data_yet_again) # changing array data does not change proxy data, or reloaded data data[:] = 42 assert_array_equal(proxy_data, proxy_copy) @@ -191,25 +203,38 @@ def validate_data(self, imaker, params): # Which unsets in_memory assert_false(img.in_memory) assert_array_equal(img.get_data(), proxy_copy) - else: # not proxy - assert_true(isinstance(img.dataobj, np.ndarray)) + # Check caching='fill' does cache data + img = imaker() + assert_false(img.in_memory) + data = img.get_data(caching='fill') assert_true(img.in_memory) - non_proxy_data = np.asarray(img.dataobj) - data = img.get_data() - assert_true(non_proxy_data is data) - # changing array data does change proxy data, and reloaded data - data[:] = 42 - assert_array_equal(np.asarray(img.dataobj), 42) - # It does change the result of get_data - assert_array_equal(img.get_data(), 42) - # Unache has no effect - img.uncache() - assert_array_equal(img.get_data(), 42) + data_again = img.get_data() + assert_true(data is data_again) + else: # not proxy + for caching in (None, 'fill', 'unchanged'): + img = imaker() + get_data_func = (img.get_data if caching is None else + partial(img.get_data, caching=caching)) + assert_true(isinstance(img.dataobj, np.ndarray)) + assert_true(img.in_memory) + data = get_data_func() + assert_true(data is img.dataobj) + # changing array data does change proxy data, and reloaded data + data[:] = 42 + assert_array_equal(np.asarray(img.dataobj), 42) + # It does change the result of get_data + assert_array_equal(get_data_func(), 42) + # Unache has no effect + img.uncache() + assert_array_equal(get_data_func(), 42) + assert_true(img.in_memory) # dataobj is read only fake_data = np.zeros(img.shape).astype(img.get_data_dtype()) assert_raises(AttributeError, setattr, img, 'dataobj', fake_data) # So is in_memory assert_raises(AttributeError, setattr, img, 'in_memory', False) + # Values to get_data caching parameter must be 'fill' or 'unchanged' + assert_raises(ValueError, img.get_data, caching='something') def validate_data_deprecated(self, imaker, params): # Check _data property still exists, but raises warning From ec90b0ad9428085bde1bade3d17d3796728e57f7 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Wed, 26 Nov 2014 17:06:19 -0800 Subject: [PATCH 4/4] RF: docstring rewrite; a PEP8 fix Suggested by Eric L from PR review. --- nibabel/spatialimages.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/nibabel/spatialimages.py b/nibabel/spatialimages.py index ace6fd0e48..4736072a79 100644 --- a/nibabel/spatialimages.py +++ b/nibabel/spatialimages.py @@ -456,6 +456,12 @@ def get_data(self, caching='fill'): store in an internal cache. Future calls to ``get_data`` will return the cached copy. + Once the data has been cached and returned from a proxy array, the + cached array can be modified by modifying the returned array, because + the returned array is a reference to the array in the cache. Regardless + of the `caching` flag, this is always true of an in-memory image (where + the image data is an array rather than an array proxy). + Parameters ---------- caching : {'fill', 'unchanged'}, optional @@ -464,16 +470,17 @@ def get_data(self, caching='fill'): is an array proxy, and the image data has not yet been cached, then 'fill' (the default) will read the data from the array proxy, and store in an internal cache, so that future calls to ``get_data`` - will return the cached copy. If 'unchanged' then leave the cache - unchanged; return the cached copy if it exists, if not, load the - data from disk and return that, but without filling the cache. + will return the cached copy. If 'unchanged' then leave the current + state of caching unchanged; return the cached copy if it exists, if + not, load the data from disk and return that, but without filling + the cache. Returns ------- data : array array of image data """ - if not caching in ('fill', 'unchanged'): + if caching not in ('fill', 'unchanged'): raise ValueError('caching value should be "fill" or "unchanged"') if self._data_cache is not None: return self._data_cache