From 6df4a95b028a7c7219ac4bff74448f5b50a04b60 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 17 Feb 2023 08:33:29 -0500 Subject: [PATCH 1/8] FIX: Disable direct creation of non-conformant GiftiDataArrays --- nibabel/gifti/gifti.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/nibabel/gifti/gifti.py b/nibabel/gifti/gifti.py index 326e60fa2..abaa81c08 100644 --- a/nibabel/gifti/gifti.py +++ b/nibabel/gifti/gifti.py @@ -460,7 +460,21 @@ def __init__( self.data = None if data is None else np.asarray(data) self.intent = intent_codes.code[intent] if datatype is None: - datatype = 'none' if self.data is None else self.data.dtype + if self.data is None: + datatype = 'none' + elif self.data.dtype in ( + np.dtype('uint8'), + np.dtype('int32'), + np.dtype('float32'), + ): + datatype = self.data.dtype + else: + raise ValueError( + f'Data array has type {self.data.dtype}. ' + 'The GIFTI standard only supports uint8, int32 and float32 arrays.\n' + 'Explicitly cast the data array to a supported dtype or pass an ' + 'explicit "datatype" parameter to GiftiDataArray().' + ) self.datatype = data_type_codes.code[datatype] self.encoding = gifti_encoding_codes.code[encoding] self.endian = gifti_endian_codes.code[endian] From b9ef70a41cdaf52d59cd2b73894f9d55443c13d1 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 17 Feb 2023 10:25:12 -0500 Subject: [PATCH 2/8] TEST: Validate GiftiDataArray construction wrt types --- nibabel/gifti/tests/test_gifti.py | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/nibabel/gifti/tests/test_gifti.py b/nibabel/gifti/tests/test_gifti.py index cd87bcfee..96fc23e61 100644 --- a/nibabel/gifti/tests/test_gifti.py +++ b/nibabel/gifti/tests/test_gifti.py @@ -195,6 +195,38 @@ def test_dataarray_init(): assert gda(ext_offset=12).ext_offset == 12 +@pytest.mark.parametrize('label', data_type_codes.value_set('label')) +def test_dataarray_typing(label): + dtype = data_type_codes.dtype[label] + code = data_type_codes.code[label] + arr = np.zeros((5,), dtype=dtype) + + # Default interface: accept standards-conformant arrays, reject else + if dtype in ('uint8', 'int32', 'float32'): + assert GiftiDataArray(arr).datatype == code + else: + with pytest.raises(ValueError): + GiftiDataArray(arr) + + # Explicit override - permit for now, may want to warn or eventually + # error + assert GiftiDataArray(arr, datatype=label).datatype == code + assert GiftiDataArray(arr, datatype=code).datatype == code + # Void is how we say we don't know how to do something, so it's not unique + if dtype != np.dtype('void'): + assert GiftiDataArray(arr, datatype=dtype).datatype == code + + # Side-load data array (as in parsing) + # We will probably always want this to load legacy images, but it's + # probably not ideal to make it easy to silently propagate nonconformant + # arrays + gda = GiftiDataArray() + gda.data = arr + gda.datatype = data_type_codes.code[label] + assert gda.data.dtype == dtype + assert gda.datatype == data_type_codes.code[label] + + def test_labeltable(): img = GiftiImage() assert len(img.labeltable.labels) == 0 From 89d20b2c23b0e8831f9a11a81d78efa372ad6ab4 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 17 Feb 2023 11:53:32 -0500 Subject: [PATCH 3/8] TEST: Upgrade to new PRNG interface and cast output when needed --- nibabel/gifti/tests/test_gifti.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nibabel/gifti/tests/test_gifti.py b/nibabel/gifti/tests/test_gifti.py index 96fc23e61..0341c571e 100644 --- a/nibabel/gifti/tests/test_gifti.py +++ b/nibabel/gifti/tests/test_gifti.py @@ -33,6 +33,8 @@ DATA_FILE6, ) +rng = np.random.default_rng() + def test_agg_data(): surf_gii_img = load(get_test_data('gifti', 'ascii.gii')) @@ -81,7 +83,7 @@ def test_gifti_image(): assert gi.numDA == 0 # Test from numpy numeric array - data = np.random.random((5,)) + data = rng.random(5, dtype=np.float32) da = GiftiDataArray(data) gi.add_gifti_data_array(da) assert gi.numDA == 1 @@ -98,7 +100,7 @@ def test_gifti_image(): # Remove one gi = GiftiImage() - da = GiftiDataArray(np.zeros((5,)), intent=0) + da = GiftiDataArray(np.zeros((5,), np.float32), intent=0) gi.add_gifti_data_array(da) gi.remove_gifti_data_array_by_intent(3) @@ -335,7 +337,7 @@ def test_metadata_list_interface(): def test_gifti_label_rgba(): - rgba = np.random.rand(4) + rgba = rng.random(4) kwargs = dict(zip(['red', 'green', 'blue', 'alpha'], rgba)) gl1 = GiftiLabel(**kwargs) From f2c108477ee3c3b1637c7c6e7876c6f3c4dc96a6 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Sat, 18 Feb 2023 14:26:32 -0500 Subject: [PATCH 4/8] ENH: Enforce GIFTI compatibility at write --- nibabel/gifti/gifti.py | 50 ++++++++++++++++++++++++------- nibabel/gifti/tests/test_gifti.py | 2 +- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/nibabel/gifti/gifti.py b/nibabel/gifti/gifti.py index abaa81c08..9dc2e42d6 100644 --- a/nibabel/gifti/gifti.py +++ b/nibabel/gifti/gifti.py @@ -16,7 +16,8 @@ import base64 import sys import warnings -from typing import Type +from copy import copy +from typing import Type, cast import numpy as np @@ -27,6 +28,12 @@ from ..nifti1 import data_type_codes, intent_codes, xform_codes from .util import KIND2FMT, array_index_order_codes, gifti_encoding_codes, gifti_endian_codes +GIFTI_DTYPES = ( + data_type_codes['NIFTI_TYPE_UINT8'], + data_type_codes['NIFTI_TYPE_INT32'], + data_type_codes['NIFTI_TYPE_FLOAT32'], +) + class _GiftiMDList(list): """List view of GiftiMetaData object that will translate most operations""" @@ -462,11 +469,7 @@ def __init__( if datatype is None: if self.data is None: datatype = 'none' - elif self.data.dtype in ( - np.dtype('uint8'), - np.dtype('int32'), - np.dtype('float32'), - ): + elif data_type_codes[self.data.dtype] in GIFTI_DTYPES: datatype = self.data.dtype else: raise ValueError( @@ -848,20 +851,45 @@ def _to_xml_element(self): GIFTI.append(dar._to_xml_element()) return GIFTI - def to_xml(self, enc='utf-8') -> bytes: + def to_xml(self, enc='utf-8', *, mode='strict') -> bytes: """Return XML corresponding to image content""" + if mode == 'strict': + if any(arr.datatype not in GIFTI_DTYPES for arr in self.darrays): + raise ValueError( + 'GiftiImage contains data arrays with invalid data types; ' + 'use mode="compat" to automatically cast to conforming types' + ) + elif mode == 'compat': + darrays = [] + for arr in self.darrays: + if arr.datatype not in GIFTI_DTYPES: + arr = copy(arr) + # TODO: Better typing for recoders + dtype = cast(np.dtype, data_type_codes.dtype[arr.datatype]) + if np.issubdtype(dtype, np.floating): + arr.datatype = data_type_codes['float32'] + elif np.issubdtype(dtype, np.integer): + arr.datatype = data_type_codes['int32'] + else: + raise ValueError(f'Cannot convert {dtype} to float32/int32') + darrays.append(arr) + gii = copy(self) + gii.darrays = darrays + return gii.to_xml(enc=enc, mode='strict') + elif mode != 'force': + raise TypeError(f'Unknown mode {mode}') header = b""" """ return header + super().to_xml(enc) # Avoid the indirection of going through to_file_map - def to_bytes(self, enc='utf-8'): - return self.to_xml(enc=enc) + def to_bytes(self, enc='utf-8', *, mode='strict'): + return self.to_xml(enc=enc, mode=mode) to_bytes.__doc__ = SerializableImage.to_bytes.__doc__ - def to_file_map(self, file_map=None, enc='utf-8'): + def to_file_map(self, file_map=None, enc='utf-8', *, mode='strict'): """Save the current image to the specified file_map Parameters @@ -877,7 +905,7 @@ def to_file_map(self, file_map=None, enc='utf-8'): if file_map is None: file_map = self.file_map with file_map['image'].get_prepare_fileobj('wb') as f: - f.write(self.to_xml(enc=enc)) + f.write(self.to_xml(enc=enc, mode=mode)) @classmethod def from_file_map(klass, file_map, buffer_size=35000000, mmap=True): diff --git a/nibabel/gifti/tests/test_gifti.py b/nibabel/gifti/tests/test_gifti.py index 0341c571e..e7050b93f 100644 --- a/nibabel/gifti/tests/test_gifti.py +++ b/nibabel/gifti/tests/test_gifti.py @@ -505,7 +505,7 @@ def test_darray_dtype_coercion_failures(): datatype=darray_dtype, ) gii = GiftiImage(darrays=[da]) - gii_copy = GiftiImage.from_bytes(gii.to_bytes()) + gii_copy = GiftiImage.from_bytes(gii.to_bytes(mode='force')) da_copy = gii_copy.darrays[0] assert np.dtype(da_copy.data.dtype) == np.dtype(darray_dtype) assert_array_equal(da_copy.data, da.data) From fead0d5dc7fcbd3f07ad5c589a045b31f658e78f Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Sat, 18 Feb 2023 14:42:28 -0500 Subject: [PATCH 5/8] DOCTEST: Catch deprecation warning in doctest --- nibabel/gifti/gifti.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nibabel/gifti/gifti.py b/nibabel/gifti/gifti.py index 9dc2e42d6..56efa4ea0 100644 --- a/nibabel/gifti/gifti.py +++ b/nibabel/gifti/gifti.py @@ -88,7 +88,8 @@ def _sanitize(args, kwargs): >>> GiftiMetaData({"key": "val"}) - >>> nvpairs = GiftiNVPairs(name='key', value='val') + >>> with pytest.deprecated_call(): + ... nvpairs = GiftiNVPairs(name='key', value='val') >>> with pytest.warns(FutureWarning): ... GiftiMetaData(nvpairs) From cf9cf150a9f2ddda7848c02c1125e12e3ddaa155 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Sun, 19 Feb 2023 16:48:35 -0500 Subject: [PATCH 6/8] TEST: Test write modes --- nibabel/gifti/tests/test_gifti.py | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/nibabel/gifti/tests/test_gifti.py b/nibabel/gifti/tests/test_gifti.py index e7050b93f..4a7b27ece 100644 --- a/nibabel/gifti/tests/test_gifti.py +++ b/nibabel/gifti/tests/test_gifti.py @@ -128,6 +128,44 @@ def assign_metadata(val): pytest.raises(TypeError, assign_metadata, 'not-a-meta') +@pytest.mark.parametrize('label', data_type_codes.value_set('label')) +def test_image_typing(label): + dtype = data_type_codes.dtype[label] + if dtype == np.void: + return + arr = 127 * rng.random( + 20, + ) + try: + cast = arr.astype(label) + except TypeError: + return + darr = GiftiDataArray(cast, datatype=label) + img = GiftiImage(darrays=[darr]) + + # Force-write always works + force_rt = img.from_bytes(img.to_bytes(mode='force')) + assert np.array_equal(cast, force_rt.darrays[0].data) + + # Compatibility mode does its best + if np.issubdtype(dtype, np.integer) or np.issubdtype(dtype, np.floating): + compat_rt = img.from_bytes(img.to_bytes(mode='compat')) + compat_darr = compat_rt.darrays[0].data + assert np.allclose(cast, compat_darr) + assert compat_darr.dtype in ('uint8', 'int32', 'float32') + else: + with pytest.raises(ValueError): + img.to_bytes(mode='compat') + + # Strict mode either works or fails + if label in ('uint8', 'int32', 'float32'): + strict_rt = img.from_bytes(img.to_bytes(mode='strict')) + assert np.array_equal(cast, strict_rt.darrays[0].data) + else: + with pytest.raises(ValueError): + img.to_bytes(mode='strict') + + def test_dataarray_empty(): # Test default initialization of DataArray null_da = GiftiDataArray() From b400dd547254083b8e27e4f0e87a899bcc6c40c8 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Sun, 19 Feb 2023 16:53:07 -0500 Subject: [PATCH 7/8] TEST: Capture stdout in some GIFTI tests --- nibabel/gifti/tests/test_gifti.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/nibabel/gifti/tests/test_gifti.py b/nibabel/gifti/tests/test_gifti.py index 4a7b27ece..d4fddf404 100644 --- a/nibabel/gifti/tests/test_gifti.py +++ b/nibabel/gifti/tests/test_gifti.py @@ -404,13 +404,17 @@ def assign_rgba(gl, val): assert np.all([elem is None for elem in gl4.rgba]) -def test_print_summary(): - for fil in [DATA_FILE1, DATA_FILE2, DATA_FILE3, DATA_FILE4, DATA_FILE5, DATA_FILE6]: - gimg = load(fil) - gimg.print_summary() +@pytest.mark.parametrize( + 'fname', [DATA_FILE1, DATA_FILE2, DATA_FILE3, DATA_FILE4, DATA_FILE5, DATA_FILE6] +) +def test_print_summary(fname, capsys): + gimg = load(fname) + gimg.print_summary() + captured = capsys.readouterr() + assert captured.out.startswith('----start----\n') -def test_gifti_coord(): +def test_gifti_coord(capsys): from ..gifti import GiftiCoordSystem gcs = GiftiCoordSystem() @@ -419,6 +423,15 @@ def test_gifti_coord(): # Smoke test gcs.xform = None gcs.print_summary() + captured = capsys.readouterr() + assert captured.out == '\n'.join( + [ + 'Dataspace: NIFTI_XFORM_UNKNOWN', + 'XFormSpace: NIFTI_XFORM_UNKNOWN', + 'Affine Transformation Matrix: ', + ' None\n', + ] + ) gcs.to_xml() From cd1a39a837b7acacf4519cb5fbf662c586c248d3 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Wed, 1 Mar 2023 18:14:55 -0500 Subject: [PATCH 8/8] Update nibabel/gifti/tests/test_gifti.py --- nibabel/gifti/tests/test_gifti.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nibabel/gifti/tests/test_gifti.py b/nibabel/gifti/tests/test_gifti.py index d4fddf404..a2f8395ca 100644 --- a/nibabel/gifti/tests/test_gifti.py +++ b/nibabel/gifti/tests/test_gifti.py @@ -133,9 +133,7 @@ def test_image_typing(label): dtype = data_type_codes.dtype[label] if dtype == np.void: return - arr = 127 * rng.random( - 20, - ) + arr = 127 * rng.random(20) try: cast = arr.astype(label) except TypeError: