Merge pull request #353 from bcipolli/gifti-fix2

RF: Deprecate get/set methods in favor of Python properties

Test consistency of data array counter
Warn on inconsistent NumberofDataArrays and DataArray elements
Remove commented code from nibabel/gifti/gifti.py

Author: effigies

nibabel/gifti/gifti.py

@@ -38,7 +38,12 @@ def from_dict(klass, data_dict):
             meda.data.append(nv)
         return meda
 
+    @np.deprecate_with_doc("Use the metadata property instead.")
     def get_metadata(self):
+        return self.metadata
+
+    @property
+    def metadata(self):
         """ Returns metadata as dictionary """
         self.data_as_dict = {}
         for ele in self.data:
@@ -59,7 +64,7 @@ def to_xml(self):
         return res
 
     def print_summary(self):
-        print(self.get_metadata())
+        print(self.metadata)
 
 
 class GiftiNVPairs(object):
@@ -128,10 +133,27 @@ def __init__(self, key=0, label='', red=None, green=None, blue=None,
         self.blue = blue
         self.alpha = alpha
 
+    @np.deprecate_with_doc("Use the rgba property instead.")
     def get_rgba(self):
+        return self.rgba
+
+    @property
+    def rgba(self):
         """ Returns RGBA as tuple """
         return (self.red, self.green, self.blue, self.alpha)
 
+    @rgba.setter
+    def rgba(self, rgba):
+        """ Set RGBA via tuple
+
+        Parameters
+        ----------
+        rgba : tuple (red, green, blue, alpha)
+
+        """
+        if len(rgba) != 4:
+            raise ValueError('rgba must be length 4.')
+        self.red, self.green, self.blue, self.alpha = rgba
 
 def _arr2txt(arr, elem_fmt):
     arr = np.asarray(arr)
@@ -338,69 +360,67 @@ def print_summary(self):
             print('Coordinate System:')
             print(self.coordsys.print_summary())
 
+    @np.deprecate_with_doc("Use the metadata property instead.")
     def get_metadata(self):
+        return self.meta.metadata
+
+    @property
+    def metadata(self):
         """ Returns metadata as dictionary """
-        return self.meta.get_metadata()
+        return self.meta.metadata
 
 
 class GiftiImage(object):
-
-    numDA = int
-    version = str
-    filename = str
-
     def __init__(self, meta=None, labeltable=None, darrays=None,
                  version="1.0"):
         if darrays is None:
             darrays = []
-        self.darrays = darrays
         if meta is None:
-            self.meta = GiftiMetaData()
-        else:
-            self.meta = meta
+            meta = GiftiMetaData()
         if labeltable is None:
-            self.labeltable = GiftiLabelTable()
-        else:
-            self.labeltable = labeltable
-        self.numDA = len(self.darrays)
+            labeltable = GiftiLabelTable()
+
+        self._labeltable = labeltable
+        self._meta = meta
+
+        self.darrays = darrays
         self.version = version
 
-#    @classmethod
-#    def from_array(cls):
-#        pass
-#def GiftiImage_fromarray(data, intent = GiftiIntentCode.NIFTI_INTENT_NONE, encoding=GiftiEncoding.GIFTI_ENCODING_B64GZ, endian = GiftiEndian.GIFTI_ENDIAN_LITTLE):
-#    """ Returns a GiftiImage from a Numpy array with a given intent code and
-#    encoding """
-
-#    @classmethod
-#    def from_vertices_and_triangles(cls):
-#        pass
-#    def from_vertices_and_triangles(cls, vertices, triangles, coordsys = None, \
-#                                    encoding = GiftiEncoding.GIFTI_ENCODING_B64GZ,\
-#                                    endian = GiftiEndian.GIFTI_ENDIAN_LITTLE):
-#    """ Returns a GiftiImage from two numpy arrays representing the vertices
-#    and the triangles. Additionally defining the coordinate system and encoding """
+    @property
+    def numDA(self):
+        return len(self.darrays)
 
-    def get_labeltable(self):
-        return self.labeltable
+    @property
+    def labeltable(self):
+        return self._labeltable
 
-    def set_labeltable(self, labeltable):
+    @labeltable.setter
+    def labeltable(self, labeltable):
         """ Set the labeltable for this GiftiImage
 
         Parameters
         ----------
         labeltable : GiftiLabelTable
 
         """
-        if isinstance(labeltable, GiftiLabelTable):
-            self.labeltable = labeltable
-        else:
-            print("Not a valid GiftiLabelTable instance")
+        if not isinstance(labeltable, GiftiLabelTable):
+            raise ValueError("Not a valid GiftiLabelTable instance")
+        self._labeltable = labeltable
 
-    def get_metadata(self):
-        return self.meta
+    @np.deprecate_with_doc("Use the gifti_img.labeltable property instead.")
+    def set_labeltable(self, labeltable):
+        self.labeltable = labeltable
 
-    def set_metadata(self, meta):
+    @np.deprecate_with_doc("Use the gifti_img.labeltable property instead.")
+    def get_labeltable(self):
+        return self.labeltable
+
+    @property
+    def meta(self):
+        return self._meta
+
+    @meta.setter
+    def meta(self, meta):
         """ Set the metadata for this GiftiImage
 
         Parameters
@@ -411,12 +431,17 @@ def set_metadata(self, meta):
         -------
         None
         """
-        if isinstance(meta, GiftiMetaData):
-            self.meta = meta
-            print("New Metadata set. Be aware of changing "
-                  "coordinate transformation!")
-        else:
-            print("Not a valid GiftiMetaData instance")
+        if not isinstance(meta, GiftiMetaData):
+            raise ValueError("Not a valid GiftiMetaData instance")
+        self._meta = meta
+
+    @np.deprecate_with_doc("Use the gifti_img.labeltable property instead.")
+    def set_metadata(self, meta):
+        self.meta = meta
+
+    @np.deprecate_with_doc("Use the gifti_img.labeltable property instead.")
+    def get_meta(self):
+        return self.meta
 
     def add_gifti_data_array(self, dataarr):
         """ Adds a data array to the GiftiImage
@@ -427,22 +452,19 @@ def add_gifti_data_array(self, dataarr):
         """
         if isinstance(dataarr, GiftiDataArray):
             self.darrays.append(dataarr)
-            self.numDA += 1
         else:
             print("dataarr paramater must be of tzpe GiftiDataArray")
 
     def remove_gifti_data_array(self, ith):
         """ Removes the ith data array element from the GiftiImage """
         self.darrays.pop(ith)
-        self.numDA -= 1
 
     def remove_gifti_data_array_by_intent(self, intent):
         """ Removes all the data arrays with the given intent type """
         intent2remove = intent_codes.code[intent]
         for dele in self.darrays:
             if dele.intent == intent2remove:
                 self.darrays.remove(dele)
-                self.numDA -= 1
 
     def get_arrays_from_intent(self, intent):
         """ Returns a a list of GiftiDataArray elements matching

nibabel/gifti/parse_gifti_fast.py

@@ -10,6 +10,7 @@
 
 import base64
 import sys
+import warnings
 import zlib
 from ..externals.six import StringIO
 from xml.parsers.expat import ParserCreate, ExpatError
@@ -109,8 +110,7 @@ def StartElementHandler(self, name, attrs):
             if 'Version' in attrs:
                 self.img.version = attrs['Version']
             if 'NumberOfDataArrays' in attrs:
-                self.img.numDA = int(attrs['NumberOfDataArrays'])
-                self.count_da = False
+                self.expected_numDA = int(attrs['NumberOfDataArrays'])
 
             self.fsm_state.append('GIFTI')
         elif name == 'MetaData':
@@ -207,6 +207,10 @@ def EndElementHandler(self, name):
         if DEBUG_PRINT:
             print('End element:\n\t', repr(name))
         if name == 'GIFTI':
+            if hasattr(self, 'expected_numDA') and self.expected_numDA != self.img.numDA:
+                warnings.warn("Actual # of data arrays does not match "
+                              "# expected: %d != %d." % (self.expected_numDA,
+                                                         self.img.numDA))
             # remove last element of the list
             self.fsm_state.pop()
             # assert len(self.fsm_state) == 0
@@ -234,8 +238,6 @@ def EndElementHandler(self, name):
             self.img.labeltable = self.lata
             self.lata = None
         elif name == 'DataArray':
-            if self.count_da:
-                self.img.numDA += 1
             self.fsm_state.pop()
         elif name == 'CoordinateSystemTransformMatrix':
             self.fsm_state.pop()

nibabel/gifti/tests/test_gifti.py

@@ -1,16 +1,17 @@
 """ Testing gifti objects
 """
+import warnings
 
 import numpy as np
 
+from ..gifti import (GiftiImage, GiftiDataArray, GiftiLabel, GiftiLabelTable,
+                     GiftiMetaData)
 from ...nifti1 import data_type_codes, intent_codes
 
-from ..gifti import GiftiImage, GiftiDataArray
-
 from numpy.testing import (assert_array_almost_equal,
                            assert_array_equal)
-
-from nose.tools import assert_true, assert_equal, assert_raises
+from nose.tools import (assert_true, assert_false, assert_equal, assert_raises)
+from ...testing import clear_and_catch_warnings
 
 
 def test_gifti_image():
@@ -24,6 +25,35 @@ def test_gifti_image():
     gi = GiftiImage()
     assert_equal(gi.darrays, [])
 
+    # Test darrays / numDA
+    gi = GiftiImage()
+    assert_equal(gi.numDA, 0)
+
+    da = GiftiDataArray(data='data')
+    gi.add_gifti_data_array(da)
+    assert_equal(gi.numDA, 1)
+    assert_equal(gi.darrays[0].data, 'data')
+
+    gi.remove_gifti_data_array(0)
+    assert_equal(gi.numDA, 0)
+
+    # Remove from empty
+    gi = GiftiImage()
+    gi.remove_gifti_data_array_by_intent(0)
+    assert_equal(gi.numDA, 0)
+
+    # Remove one
+    gi = GiftiImage()
+    da = GiftiDataArray(data='data')
+    gi.add_gifti_data_array(da)
+
+    gi.remove_gifti_data_array_by_intent(0)
+    assert_equal(gi.numDA, 1)
+
+    gi.darrays[0].intent = 0
+    gi.remove_gifti_data_array_by_intent(0)
+    assert_equal(gi.numDA, 0)
+
 
 def test_dataarray():
     for dt_code in data_type_codes.value_set():
@@ -36,3 +66,64 @@ def test_dataarray():
         bs_arr = arr.byteswap().newbyteorder()
         da = GiftiDataArray.from_array(bs_arr, 'triangle')
         assert_equal(da.datatype, data_type_codes[arr.dtype])
+
+
+def test_labeltable():
+    img = GiftiImage()
+    assert_equal(len(img.labeltable.labels), 0)
+
+    new_table = GiftiLabelTable()
+    new_table.labels += ['test', 'me']
+    img.labeltable = new_table
+    assert_equal(len(img.labeltable.labels), 2)
+
+    # Try to set to non-table
+    def assign_labeltable(val):
+        img.labeltable = val
+    assert_raises(ValueError, assign_labeltable, 'not-a-table')
+
+
+def test_metadata():
+    # Test deprecation
+    with clear_and_catch_warnings() as w:
+        warnings.filterwarnings('once', category=DeprecationWarning)
+        assert_equal(len(GiftiDataArray().get_metadata()), 0)
+
+    # Test deprecation
+    with clear_and_catch_warnings() as w:
+        warnings.filterwarnings('once', category=DeprecationWarning)
+        assert_equal(len(GiftiMetaData().get_metadata()), 0)
+
+
+def test_gifti_label_rgba():
+    rgba = np.random.rand(4)
+    kwargs = dict(zip(['red', 'green', 'blue', 'alpha'], rgba))
+
+    gl1 = GiftiLabel(**kwargs)
+    assert_array_equal(rgba, gl1.rgba)
+
+    gl1.red = 2 * gl1.red
+    assert_false(np.allclose(rgba, gl1.rgba))  # don't just store the list!
+
+    gl2 = GiftiLabel()
+    gl2.rgba = rgba
+    assert_array_equal(rgba, gl2.rgba)
+
+    gl2.blue = 2 * gl2.blue
+    assert_false(np.allclose(rgba, gl2.rgba))  # don't just store the list!
+
+    def assign_rgba(gl, val):
+        gl.rgba = val
+    gl3 = GiftiLabel(**kwargs)
+    assert_raises(ValueError, assign_rgba, gl3, rgba[:2])
+    assert_raises(ValueError, assign_rgba, gl3, rgba.tolist() + rgba.tolist())
+
+    # Test deprecation
+    with clear_and_catch_warnings() as w:
+        warnings.filterwarnings('once', category=DeprecationWarning)
+        assert_equal(kwargs['red'], gl3.get_rgba()[0])
+
+    # Test default value
+    gl4 = GiftiLabel()
+    assert_equal(len(gl4.rgba), 4)
+    assert_true(np.all([elem is None for elem in gl4.rgba]))