Merge pull request #470 from matthew-brett/gifti-serialization-bug

MRG: fix in-memory GIFTI XML output, drop python 2.6

Fix errors writing XML output from GIFTI images created in-memory (rather than loaded).

Also, drop Python 2.6 testing and compatibility.

Author: matthew-brett

.travis.yml

@@ -24,7 +24,6 @@ env:
         - PYDICOM=1
         - INSTALL_TYPE="setup"
 python:
-    - 2.6
     - 3.3
     - 3.4
     - 3.5
@@ -38,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

doc/source/installation.rst

@@ -81,7 +81,12 @@ is for you.
 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_ (optional, for full SPM-ANALYZE support)
 *  PyDICOM_ 0.9.7 or greater (optional, for DICOM support)

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

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.
@@ -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)
 
@@ -517,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:
 
@@ -553,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:

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):
@@ -187,7 +192,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 = _str2int(attrs["ExternalFileOffset"])
             self.img.darrays.append(self.da)
             self.fsm_state.append('DataArray')
 

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'''<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE GIFTI SYSTEM "http://www.nitrc.org/frs/download.php/1594/gifti.dtd">
+<GIFTI
+xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+xsi:noNamespaceSchemaLocation="http://www.nitrc.org/frs/download.php/1303/GIFTI_Caret.xsd"
+Version="1.0"
+NumberOfDataArrays="2">
+<MetaData>
+<MD>
+<Name><![CDATA[date]]></Name>
+<Value><![CDATA[Thu Nov 15 09:05:22 2007]]></Value>
+</MD>
+</MetaData>
+<LabelTable/>
+<DataArray Intent="NIFTI_INTENT_POINTSET"
+DataType="NIFTI_TYPE_FLOAT32"
+ArrayIndexingOrder="RowMajorOrder"
+Dimensionality="2"
+Dim0="4"
+Dim1="3"
+Encoding="ASCII"
+Endian="LittleEndian"
+ExternalFileName=""
+ExternalFileOffset="">
+<CoordinateSystemTransformMatrix>
+<DataSpace><![CDATA[NIFTI_XFORM_TALAIRACH]]></DataSpace>
+<TransformedSpace><![CDATA[NIFTI_XFORM_TALAIRACH]]></TransformedSpace>
+<MatrixData>
+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
+</MatrixData>
+</CoordinateSystemTransformMatrix>
+<Data>
+10.5 0 0
+0 20.5 0
+0 0 30.5
+0 0 0
+</Data>
+</DataArray>
+<DataArray Intent="NIFTI_INTENT_TRIANGLE"
+DataType="NIFTI_TYPE_INT32"
+ArrayIndexingOrder="RowMajorOrder"
+Dimensionality="2"
+Dim0="4"
+Dim1="3"
+Encoding="ASCII"
+Endian="LittleEndian"
+ExternalFileName="" ExternalFileOffset="">
+<Data>
+0 1 2
+1 2 3
+0 1 3
+0 2 3
+</Data>
+</DataArray>
+</GIFTI>'''
+
+    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)

nibabel/gifti/tests/test_parse_gifti_fast.py

@@ -99,6 +99,45 @@
 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_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
+        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

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

requirements.txt

@@ -1 +1,8 @@
+# Minumum requirements
+#
+# Check these against
+#   nibabel/info.py
+#   .travis.yml
+#   doc/source/installation.rst
+
 numpy>=1.5.1