diff --git a/nibabel/arrayproxy.py b/nibabel/arrayproxy.py index 6cda3a206a..dc9b171c0b 100644 --- a/nibabel/arrayproxy.py +++ b/nibabel/arrayproxy.py @@ -27,6 +27,7 @@ """ from contextlib import contextmanager from threading import RLock +import warnings import numpy as np @@ -53,7 +54,7 @@ KEEP_FILE_OPEN_DEFAULT = False -class ArrayProxy(object): +class ArrayProxy: """ Class to act as proxy for the array that can be read from a file The array proxy allows us to freeze the passed fileobj and header such that @@ -83,10 +84,9 @@ class ArrayProxy(object): See :mod:`nibabel.minc1`, :mod:`nibabel.ecat` and :mod:`nibabel.parrec` for examples. """ - # Assume Fortran array memory layout - order = 'F' + _default_order = 'F' - def __init__(self, file_like, spec, *, mmap=True, keep_file_open=None): + def __init__(self, file_like, spec, *, mmap=True, order=None, keep_file_open=None): """Initialize array proxy instance Parameters @@ -116,6 +116,11 @@ def __init__(self, file_like, spec, *, mmap=True, keep_file_open=None): True gives the same behavior as ``mmap='c'``. If `file_like` cannot be memory-mapped, ignore `mmap` value and read array from file. + order : {None, 'F', 'C'}, optional, keyword only + `order` controls the order of the data array layout. Fortran-style, + column-major order may be indicated with 'F', and C-style, row-major + order may be indicated with 'C'. None gives the default order, that + comes from the `_default_order` class variable. keep_file_open : { None, True, False }, optional, keyword only `keep_file_open` controls whether a new file handle is created every time the image is accessed, or a single file handle is @@ -128,6 +133,8 @@ def __init__(self, file_like, spec, *, mmap=True, keep_file_open=None): """ if mmap not in (True, False, 'c', 'r'): raise ValueError("mmap should be one of {True, False, 'c', 'r'}") + if order not in (None, 'C', 'F'): + raise ValueError("order should be one of {None, 'C', 'F'}") self.file_like = file_like if hasattr(spec, 'get_data_shape'): slope, inter = spec.get_slope_inter() @@ -142,11 +149,25 @@ def __init__(self, file_like, spec, *, mmap=True, keep_file_open=None): else: raise TypeError('spec must be tuple of length 2-5 or header object') + # Warn downstream users that the class variable order is going away + if hasattr(self.__class__, 'order'): + warnings.warn(f'Class {self.__class__} has an `order` class variable. ' + 'ArrayProxy subclasses should rename this variable to `_default_order` ' + 'to avoid conflict with instance variables.\n' + '* deprecated in version: 5.0\n' + '* will raise error in version: 7.0\n', + DeprecationWarning, stacklevel=2) + # Override _default_order with order, to follow intent of subclasser + self._default_order = self.order + # Copies of values needed to read array self._shape, self._dtype, self._offset, self._slope, self._inter = par # Permit any specifier that can be interpreted as a numpy dtype self._dtype = np.dtype(self._dtype) self._mmap = mmap + if order is None: + order = self._default_order + self.order = order # Flags to keep track of whether a single ImageOpener is created, and # whether a single underlying file handle is created. self._keep_file_open, self._persist_opener = \ diff --git a/nibabel/tests/test_arrayproxy.py b/nibabel/tests/test_arrayproxy.py index 80806cae3a..ed89105aa0 100644 --- a/nibabel/tests/test_arrayproxy.py +++ b/nibabel/tests/test_arrayproxy.py @@ -15,13 +15,16 @@ import pickle from io import BytesIO +from packaging.version import Version from ..tmpdirs import InTemporaryDirectory import numpy as np +from .. import __version__ from ..arrayproxy import (ArrayProxy, is_proxy, reshape_dataobj, get_obj_dtype) from ..openers import ImageOpener from ..nifti1 import Nifti1Header +from ..deprecator import ExpiredDeprecationError from unittest import mock @@ -57,6 +60,11 @@ def copy(self): class CArrayProxy(ArrayProxy): # C array memory layout + _default_order = 'C' + + +class DeprecatedCArrayProxy(ArrayProxy): + # Used in test_deprecated_order_classvar. Remove when that test is removed (8.0) order = 'C' @@ -81,6 +89,9 @@ def test_init(): assert ap.shape != shape # Data stays the same, also assert_array_equal(np.asarray(ap), arr) + # You wouldn't do this, but order=None explicitly requests the default order + ap2 = ArrayProxy(bio, FunkyHeader(arr.shape), order=None) + assert_array_equal(np.asarray(ap2), arr) # C order also possible bio = BytesIO() bio.seek(16) @@ -90,6 +101,8 @@ def test_init(): # Illegal init with pytest.raises(TypeError): ArrayProxy(bio, object()) + with pytest.raises(ValueError): + ArrayProxy(bio, hdr, order='badval') def test_tuplespec(): @@ -154,33 +167,87 @@ def test_nifti1_init(): assert_array_equal(np.asarray(ap), arr * 2.0 + 10) -def test_proxy_slicing(): - shapes = (15, 16, 17) - for n_dim in range(1, len(shapes) + 1): - shape = shapes[:n_dim] - arr = np.arange(np.prod(shape)).reshape(shape) - for offset in (0, 20): - hdr = Nifti1Header() - hdr.set_data_offset(offset) - hdr.set_data_dtype(arr.dtype) - hdr.set_data_shape(shape) - for order, klass in ('F', ArrayProxy), ('C', CArrayProxy): - fobj = BytesIO() - fobj.write(b'\0' * offset) - fobj.write(arr.tobytes(order=order)) - prox = klass(fobj, hdr) - for sliceobj in slicer_samples(shape): - assert_array_equal(arr[sliceobj], prox[sliceobj]) - # Check slicing works with scaling +@pytest.mark.parametrize("n_dim", (1, 2, 3)) +@pytest.mark.parametrize("offset", (0, 20)) +def test_proxy_slicing(n_dim, offset): + shape = (15, 16, 17)[:n_dim] + arr = np.arange(np.prod(shape)).reshape(shape) + hdr = Nifti1Header() + hdr.set_data_offset(offset) + hdr.set_data_dtype(arr.dtype) + hdr.set_data_shape(shape) + for order, klass in ('F', ArrayProxy), ('C', CArrayProxy): + fobj = BytesIO() + fobj.write(b'\0' * offset) + fobj.write(arr.tobytes(order=order)) + prox = klass(fobj, hdr) + assert prox.order == order + for sliceobj in slicer_samples(shape): + assert_array_equal(arr[sliceobj], prox[sliceobj]) + + +def test_proxy_slicing_with_scaling(): + shape = (15, 16, 17) + offset = 20 + arr = np.arange(np.prod(shape)).reshape(shape) + hdr = Nifti1Header() + hdr.set_data_offset(offset) + hdr.set_data_dtype(arr.dtype) + hdr.set_data_shape(shape) hdr.set_slope_inter(2.0, 1.0) fobj = BytesIO() - fobj.write(b'\0' * offset) + fobj.write(bytes(offset)) fobj.write(arr.tobytes(order='F')) prox = ArrayProxy(fobj, hdr) sliceobj = (None, slice(None), 1, -1) assert_array_equal(arr[sliceobj] * 2.0 + 1.0, prox[sliceobj]) +@pytest.mark.parametrize("order", ("C", "F")) +def test_order_override(order): + shape = (15, 16, 17) + arr = np.arange(np.prod(shape)).reshape(shape) + fobj = BytesIO() + fobj.write(arr.tobytes(order=order)) + for klass in (ArrayProxy, CArrayProxy): + prox = klass(fobj, (shape, arr.dtype), order=order) + assert prox.order == order + sliceobj = (None, slice(None), 1, -1) + assert_array_equal(arr[sliceobj], prox[sliceobj]) + + +def test_deprecated_order_classvar(): + shape = (15, 16, 17) + arr = np.arange(np.prod(shape)).reshape(shape) + fobj = BytesIO() + fobj.write(arr.tobytes(order='C')) + sliceobj = (None, slice(None), 1, -1) + + # We don't really care about the original order, just that the behavior + # of the deprecated mode matches the new behavior + fprox = ArrayProxy(fobj, (shape, arr.dtype), order='F') + cprox = ArrayProxy(fobj, (shape, arr.dtype), order='C') + + # Start raising errors when we crank the dev version + if Version(__version__) >= Version('7.0.0.dev0'): + cm = pytest.raises(ExpiredDeprecationError) + else: + cm = pytest.deprecated_call() + + with cm: + prox = DeprecatedCArrayProxy(fobj, (shape, arr.dtype)) + assert prox.order == 'C' + assert_array_equal(prox[sliceobj], cprox[sliceobj]) + with cm: + prox = DeprecatedCArrayProxy(fobj, (shape, arr.dtype), order='C') + assert prox.order == 'C' + assert_array_equal(prox[sliceobj], cprox[sliceobj]) + with cm: + prox = DeprecatedCArrayProxy(fobj, (shape, arr.dtype), order='F') + assert prox.order == 'F' + assert_array_equal(prox[sliceobj], fprox[sliceobj]) + + def test_is_proxy(): # Test is_proxy function hdr = FunkyHeader((2, 3, 4))