From 5f632fe564377219592ad1f994b2b99146e265d8 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Tue, 2 Aug 2016 17:10:20 -0700 Subject: [PATCH 1/8] BF+TST: fix XML output from in-memory Gifti array GiftiDataArrays created in memory (rather than loaded from disk) had ExternalFileOffset attribute value as an integer, but for valid XML, the values have to be strings. Closes gh-469. --- nibabel/gifti/gifti.py | 3 +- nibabel/gifti/tests/test_gifti.py | 113 ++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/nibabel/gifti/gifti.py b/nibabel/gifti/gifti.py index dd330b123e..d45fddf6b9 100644 --- a/nibabel/gifti/gifti.py +++ b/nibabel/gifti/gifti.py @@ -436,6 +436,7 @@ def _to_xml_element(self): # fix endianness to machine endianness self.endian = gifti_endian_codes.code[sys.byteorder] + # All attribute values must be strings data_array = xml.Element('DataArray', attrib={ 'Intent': intent_codes.niistring[self.intent], 'DataType': data_type_codes.niistring[self.datatype], @@ -444,7 +445,7 @@ def _to_xml_element(self): 'Encoding': gifti_encoding_codes.specs[self.encoding], 'Endian': gifti_endian_codes.specs[self.endian], 'ExternalFileName': self.ext_fname, - 'ExternalFileOffset': self.ext_offset}) + 'ExternalFileOffset': str(self.ext_offset)}) for di, dn in enumerate(self.dims): data_array.attrib['Dim%d' % di] = str(dn) diff --git a/nibabel/gifti/tests/test_gifti.py b/nibabel/gifti/tests/test_gifti.py index 4865353e00..baf0518384 100644 --- a/nibabel/gifti/tests/test_gifti.py +++ b/nibabel/gifti/tests/test_gifti.py @@ -2,6 +2,7 @@ """ import warnings import sys +from io import BytesIO import numpy as np @@ -12,6 +13,7 @@ GiftiCoordSystem) from nibabel.gifti.gifti import data_tag from nibabel.nifti1 import data_type_codes +from nibabel.fileholders import FileHolder from numpy.testing import (assert_array_almost_equal, assert_array_equal) @@ -275,3 +277,114 @@ def test_data_tag_deprecated(): warnings.filterwarnings('once', category=DeprecationWarning) data_tag(np.array([]), 'ASCII', '%i', 1) assert_equal(len(w), 1) + + +def test_gifti_round_trip(): + # From section 14.4 in GIFTI Surface Data Format Version 1.0 + # (with some adaptations) + + test_data = b''' + + + + + + + + + + + + + + +1.000000 0.000000 0.000000 0.000000 +0.000000 1.000000 0.000000 0.000000 +0.000000 0.000000 1.000000 0.000000 +0.000000 0.000000 0.000000 1.000000 + + + +10.5 0 0 +0 20.5 0 +0 0 30.5 +0 0 0 + + + + +0 1 2 +1 2 3 +0 1 3 +0 2 3 + + +''' + + exp_verts = np.zeros((4, 3)) + exp_verts[0, 0] = 10.5 + exp_verts[1, 1] = 20.5 + exp_verts[2, 2] = 30.5 + exp_faces = np.asarray([[0, 1, 2], [1, 2, 3], [0, 1, 3], [0, 2, 3]], + dtype=np.int32) + + def _check_gifti(gio): + vertices = gio.get_arrays_from_intent('NIFTI_INTENT_POINTSET')[0].data + faces = gio.get_arrays_from_intent('NIFTI_INTENT_TRIANGLE')[0].data + assert_array_equal(vertices, exp_verts) + assert_array_equal(faces, exp_faces) + + bio = BytesIO() + fmap = dict(image=FileHolder(fileobj=bio)) + + bio.write(test_data) + bio.seek(0) + gio = GiftiImage.from_file_map(fmap) + _check_gifti(gio) + # Write and read again + bio.seek(0) + gio.to_file_map(fmap) + bio.seek(0) + gio2 = GiftiImage.from_file_map(fmap) + _check_gifti(gio2) + + +def test_data_array_round_trip(): + # Test valid XML generated from new in-memory array + # See: https://github.com/nipy/nibabel/issues/469 + verts = np.zeros((4, 3), np.float32) + verts[0, 0] = 10.5 + verts[1, 1] = 20.5 + verts[2, 2] = 30.5 + + vertices = GiftiDataArray(verts) + img = GiftiImage() + img.add_gifti_data_array(vertices) + bio = BytesIO() + fmap = dict(image=FileHolder(fileobj=bio)) + bio.write(img.to_xml()) + bio.seek(0) + gio = GiftiImage.from_file_map(fmap) + vertices = gio.darrays[0].data + assert_array_equal(vertices, verts) From 32271df8b9bc4144ba6b5450a6f91c2de20226e7 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 6 Aug 2016 14:16:14 -0400 Subject: [PATCH 2/8] BF: Coerce parsed GiftiDataArray.ext_offset to int --- nibabel/gifti/parse_gifti_fast.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nibabel/gifti/parse_gifti_fast.py b/nibabel/gifti/parse_gifti_fast.py index 1821ad2d4d..896be689e5 100644 --- a/nibabel/gifti/parse_gifti_fast.py +++ b/nibabel/gifti/parse_gifti_fast.py @@ -187,7 +187,7 @@ def StartElementHandler(self, name, attrs): if "ExternalFileName" in attrs: self.da.ext_fname = attrs["ExternalFileName"] if "ExternalFileOffset" in attrs: - self.da.ext_offset = attrs["ExternalFileOffset"] + self.da.ext_offset = int(attrs["ExternalFileOffset"]) self.img.darrays.append(self.da) self.fsm_state.append('DataArray') From fdec48bda909874291a7cb69a39b581ba1d689dd Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 6 Aug 2016 14:46:22 -0400 Subject: [PATCH 3/8] TEST: Test default types for parsed Gifti files --- nibabel/gifti/tests/test_parse_gifti_fast.py | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/nibabel/gifti/tests/test_parse_gifti_fast.py b/nibabel/gifti/tests/test_parse_gifti_fast.py index d3a425daa9..bfbac84a2f 100644 --- a/nibabel/gifti/tests/test_parse_gifti_fast.py +++ b/nibabel/gifti/tests/test_parse_gifti_fast.py @@ -99,6 +99,44 @@ DATA_FILE6_darr1 = np.array([9182740, 9182740, 9182740], dtype=np.float32) +def assert_default_types(loaded): + default = loaded.__class__() + for attr in dir(default): + defaulttype = type(getattr(default, attr)) + # Optional elements may have default of None + if defaulttype is type(None): + continue + loadedtype = type(getattr(loaded, attr)) + assert loadedtype == defaulttype, \ + "Type mismatch for attribute: {} ({!s} != {!s})".format( + attr, loadedtype, defaulttype) + + +def test_default_types(): + for fname in datafiles: + img = load(fname) + # GiftiImage + assert_default_types(img) + # GiftiMetaData + assert_default_types(img.meta) + # GiftiNVPairs + for nvpair in img.meta.data: + assert_default_types(nvpair) + # GiftiLabelTable + assert_default_types(img.labeltable) + # GiftiLabel elements can be None or float; skip + # GiftiDataArray + for darray in img.darrays: + assert_default_types(darray) + # GiftiCoordSystem + assert_default_types(darray.coordsys) + # GiftiMetaData + assert_default_types(darray.meta) + # GiftiNVPairs + for nvpair in darray.meta.data: + assert_default_types(nvpair) + + def test_read_ordering(): # DATA_FILE1 has an expected darray[0].data shape of (3,3). However if we # read another image first (DATA_FILE2) then the shape is wrong From aa4db53c99cd2ef8821288269a5267deef169e99 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 6 Aug 2016 14:47:52 -0400 Subject: [PATCH 4/8] BF: Handle empty strings in ExternalFileOffset --- nibabel/gifti/parse_gifti_fast.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nibabel/gifti/parse_gifti_fast.py b/nibabel/gifti/parse_gifti_fast.py index 896be689e5..e0307768f7 100644 --- a/nibabel/gifti/parse_gifti_fast.py +++ b/nibabel/gifti/parse_gifti_fast.py @@ -186,7 +186,7 @@ def StartElementHandler(self, name, attrs): self.da.endian = gifti_endian_codes.code[attrs["Endian"]] if "ExternalFileName" in attrs: self.da.ext_fname = attrs["ExternalFileName"] - if "ExternalFileOffset" in attrs: + if "ExternalFileOffset" in attrs and attrs["ExternalFileOffset"]: self.da.ext_offset = int(attrs["ExternalFileOffset"]) self.img.darrays.append(self.da) self.fsm_state.append('DataArray') From 4b1a55cc777ee87d17e6c2ba04fa8697fa2980c2 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Sat, 6 Aug 2016 12:54:51 -0700 Subject: [PATCH 5/8] RF: helper function for converting str to int Add / use helper function for converting string to integer, where empty string maps to 0. --- nibabel/gifti/parse_gifti_fast.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/nibabel/gifti/parse_gifti_fast.py b/nibabel/gifti/parse_gifti_fast.py index e0307768f7..e0eba10350 100644 --- a/nibabel/gifti/parse_gifti_fast.py +++ b/nibabel/gifti/parse_gifti_fast.py @@ -77,6 +77,11 @@ def read_data_block(encoding, endian, ordering, datatype, shape, data): return newarr +def _str2int(in_str): + # Convert string to integer, where empty string gives 0 + return int(in_str) if in_str else 0 + + class GiftiImageParser(XmlParser): def __init__(self, encoding=None, buffer_size=35000000, verbose=0): @@ -186,8 +191,8 @@ def StartElementHandler(self, name, attrs): self.da.endian = gifti_endian_codes.code[attrs["Endian"]] if "ExternalFileName" in attrs: self.da.ext_fname = attrs["ExternalFileName"] - if "ExternalFileOffset" in attrs and attrs["ExternalFileOffset"]: - self.da.ext_offset = int(attrs["ExternalFileOffset"]) + if "ExternalFileOffset" in attrs: + self.da.ext_offset = _str2int(attrs["ExternalFileOffset"]) self.img.darrays.append(self.da) self.fsm_state.append('DataArray') From 61c9005f254e9921e76cfba6b630294cfa32ec0f Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Sat, 6 Aug 2016 12:55:39 -0700 Subject: [PATCH 6/8] RF: set default GIFTI strings as unicode Specify all default GIFTI strings as unicode. --- nibabel/gifti/gifti.py | 9 +++++---- nibabel/gifti/tests/test_parse_gifti_fast.py | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/nibabel/gifti/gifti.py b/nibabel/gifti/gifti.py index d45fddf6b9..68d0faea6d 100644 --- a/nibabel/gifti/gifti.py +++ b/nibabel/gifti/gifti.py @@ -81,7 +81,7 @@ class GiftiNVPairs(object): name : str value : str """ - def __init__(self, name='', value=''): + def __init__(self, name=u'', value=u''): self.name = name self.value = value @@ -344,7 +344,7 @@ def __init__(self, coordsys=None, ordering="C", meta=None, - ext_fname='', + ext_fname=u'', ext_offset=0): """ Returns a shell object that cannot be saved. @@ -518,7 +518,8 @@ def metadata(self): class GiftiImage(xml.XmlSerializable, FileBasedImage): - """ + """ GIFTI image object + The Gifti spec suggests using the following suffixes to your filename when saving each specific type of data: @@ -554,7 +555,7 @@ class GiftiImage(xml.XmlSerializable, FileBasedImage): parser = None def __init__(self, header=None, extra=None, file_map=None, meta=None, - labeltable=None, darrays=None, version="1.0"): + labeltable=None, darrays=None, version=u"1.0"): super(GiftiImage, self).__init__(header=header, extra=extra, file_map=file_map) if darrays is None: diff --git a/nibabel/gifti/tests/test_parse_gifti_fast.py b/nibabel/gifti/tests/test_parse_gifti_fast.py index bfbac84a2f..9adc03d8fd 100644 --- a/nibabel/gifti/tests/test_parse_gifti_fast.py +++ b/nibabel/gifti/tests/test_parse_gifti_fast.py @@ -107,12 +107,13 @@ def assert_default_types(loaded): if defaulttype is type(None): continue loadedtype = type(getattr(loaded, attr)) - assert loadedtype == defaulttype, \ - "Type mismatch for attribute: {} ({!s} != {!s})".format( - attr, loadedtype, defaulttype) + assert_equal(loadedtype, defaulttype, + "Type mismatch for attribute: {} ({!s} != {!s})".format( + attr, loadedtype, defaulttype)) def test_default_types(): + # Test that variable types are same in loaded and default instances for fname in datafiles: img = load(fname) # GiftiImage From 529150cc740600539f26cadbbc313233f65b2bcd Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Sat, 6 Aug 2016 13:40:27 -0700 Subject: [PATCH 7/8] MAINT: drop Python 2.6 tests Goodbye old friend. Thanks for all the fish. --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 5e4fc0389e..28331293fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,7 +24,6 @@ env: - PYDICOM=1 - INSTALL_TYPE="setup" python: - - 2.6 - 3.3 - 3.4 - 3.5 From 5dad677bfd6c9fe46148c70e0a9b6c429026480a Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Sat, 6 Aug 2016 13:50:44 -0700 Subject: [PATCH 8/8] DOC: better documentation requirement checks Note where requirements should be updated. Update minimum numpy version. Note new minimum Python version (2.7). --- .travis.yml | 4 ++++ doc/source/installation.rst | 7 ++++++- doc/source/links_names.txt | 2 +- nibabel/info.py | 7 +++++-- requirements.txt | 7 +++++++ 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 28331293fa..da94d4e662 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,6 +37,10 @@ matrix: env: - DEPENDS=numpy==1.5.1 PYDICOM=0 # Absolute minimum dependencies plus oldest MPL + # Check these against: + # doc/source/installation.rst + # requirements.txt + # .travis.yml - python: 2.7 env: - DEPENDS="numpy==1.5.1 matplotlib==1.3.1" PYDICOM=0 diff --git a/doc/source/installation.rst b/doc/source/installation.rst index 0d1df68343..bbd6134623 100644 --- a/doc/source/installation.rst +++ b/doc/source/installation.rst @@ -75,7 +75,12 @@ NiBabel from source. Requirements ------------ -* Python_ 2.6 or greater +.. check these against: + nibabel/info.py + requirements.txt + .travis.yml + +* Python_ 2.7 or greater * NumPy_ 1.5 or greater * SciPy_ (for full SPM-ANALYZE support) * PyDICOM_ 0.9.7 or greater (for DICOM support) diff --git a/doc/source/links_names.txt b/doc/source/links_names.txt index 1cab6fd63f..62358b4625 100644 --- a/doc/source/links_names.txt +++ b/doc/source/links_names.txt @@ -108,7 +108,7 @@ https://pip.readthedocs.org/en/latest/installing.html .. _twine: https://pypi.python.org/pypi/twine .. _datapkg: https://pythonhosted.org/datapkg/ -.. _python imaging library: http://pythonware.com/products/pil/ +.. _python imaging library: https://pypi.python.org/pypi/Pillow .. Python imaging projects .. _PyMVPA: http://www.pymvpa.org diff --git a/nibabel/info.py b/nibabel/info.py index 1e78318f2b..f830c35e2a 100644 --- a/nibabel/info.py +++ b/nibabel/info.py @@ -102,8 +102,11 @@ nibabel distribution. """ -# versions for dependencies -NUMPY_MIN_VERSION = '1.5' +# versions for dependencies. Check these against: +# doc/source/installation.rst +# requirements.txt +# .travis.yml +NUMPY_MIN_VERSION = '1.5.1' PYDICOM_MIN_VERSION = '0.9.7' # Main setup parameters diff --git a/requirements.txt b/requirements.txt index d37709b4c0..28c0ee779a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,8 @@ +# Minumum requirements +# +# Check these against +# nibabel/info.py +# .travis.yml +# doc/source/installation.rst + numpy>=1.5.1