Merge pull request #1127 from effigies/fix/gifti_md_list

FIX: Make GiftiMetaData.data a list proxy, deprecate

Author: effigies

nibabel/gifti/gifti.py

@@ -26,6 +26,42 @@
 from ..deprecated import deprecate_with_version
 
 
+class _GiftiMDList(list):
+    """List view of GiftiMetaData object that will translate most operations"""
+    def __init__(self, metadata):
+        self._md = metadata
+        super().__init__(
+            GiftiNVPairs._private_init(k, v, metadata)
+            for k, v in metadata.items()
+        )
+
+    def append(self, nvpair):
+        self._md[nvpair.name] = nvpair.value
+        super().append(nvpair)
+
+    def clear(self):
+        super().clear()
+        self._md.clear()
+
+    def extend(self, iterable):
+        for nvpair in iterable:
+            self.append(nvpair)
+
+    def insert(self, index, nvpair):
+        self._md[nvpair.name] = nvpair.value
+        super().insert(index, nvpair)
+
+    def pop(self, index=-1):
+        nvpair = super().pop(index)
+        nvpair._container = None
+        del self._md[nvpair.name]
+        return nvpair
+
+    def remove(self, nvpair):
+        super().remove(nvpair)
+        del self._md[nvpair.name]
+
+
 class GiftiMetaData(CaretMetaData):
     """ A sequence of GiftiNVPairs containing metadata for a gifti data array
     """
@@ -72,11 +108,12 @@ def _sanitize(args, kwargs):
         return (), {pair.name: pair.value}
 
     @property
+    @deprecate_with_version(
+        'The data attribute is deprecated. Use GiftiMetaData object '
+        'directly as a dict.',
+        '4.0', '6.0')
     def data(self):
-        warnings.warn(
-            "GiftiMetaData.data will be a dict in NiBabel 6.0.",
-            FutureWarning, stacklevel=2)
-        return [GiftiNVPairs(k, v) for k, v in self._data.items()]
+        return _GiftiMDList(self)
 
     @classmethod
     @deprecate_with_version(
@@ -94,7 +131,7 @@ def get_metadata(self):
 
     @property
     @deprecate_with_version(
-        'metadata property deprecated. Use GiftiMetadata object '
+        'metadata property deprecated. Use GiftiMetaData object '
         'as dict or pass to dict() for a standard dictionary.',
         '4.0', '6.0')
     def metadata(self):
@@ -113,9 +150,48 @@ class GiftiNVPairs:
     name : str
     value : str
     """
+    @deprecate_with_version(
+        'GiftiNVPairs objects are deprecated. Use the GiftiMetaData object '
+        'as a dict, instead.',
+        '4.0', '6.0')
     def __init__(self, name=u'', value=u''):
-        self.name = name
-        self.value = value
+        self._name = name
+        self._value = value
+        self._container = None
+
+    @classmethod
+    def _private_init(cls, name, value, md):
+        """Private init method to provide warning-free experience"""
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore', DeprecationWarning)
+            self = cls(name, value)
+        self._container = md
+        return self
+
+    def __eq__(self, other):
+        if not isinstance(other, GiftiNVPairs):
+            return NotImplemented
+        return self.name == other.name and self.value == other.value
+
+    @property
+    def name(self):
+        return self._name
+
+    @name.setter
+    def name(self, key):
+        if self._container:
+            self._container[key] = self._container.pop(self._name)
+        self._name = key
+
+    @property
+    def value(self):
+        return self._value
+
+    @value.setter
+    def value(self, val):
+        if self._container:
+            self._container[self._name] = val
+        self._value = val
 
 
 class GiftiLabelTable(xml.XmlSerializable):

nibabel/gifti/tests/test_gifti.py

@@ -228,15 +228,16 @@ def test_labeltable():
 def test_metadata():
     md = GiftiMetaData(key='value')
     # Old initialization methods
-    nvpair = GiftiNVPairs('key', 'value')
+    with pytest.warns(DeprecationWarning) as w:
+        nvpair = GiftiNVPairs('key', 'value')
     with pytest.warns(FutureWarning) as w:
         md2 = GiftiMetaData(nvpair=nvpair)
     assert len(w) == 1
     with pytest.warns(DeprecationWarning) as w:
         md3 = GiftiMetaData.from_dict({'key': 'value'})
     assert md == md2 == md3 == {'key': 'value'}
     # .data as a list of NVPairs is going away
-    with pytest.warns(FutureWarning) as w:
+    with pytest.warns(DeprecationWarning) as w:
         assert md.data[0].name == 'key'
         assert md.data[0].value == 'value'
     assert len(w) == 2
@@ -245,6 +246,85 @@ def test_metadata():
         md.get_metadata()
 
 
+def test_metadata_list_interface():
+    md = GiftiMetaData(key='value')
+    with pytest.warns(DeprecationWarning):
+        mdlist = md.data
+    assert len(mdlist) == 1
+    assert mdlist[0].name == 'key'
+    assert mdlist[0].value == 'value'
+
+    # Modify elements in-place
+    mdlist[0].name = 'foo'
+    assert mdlist[0].name == 'foo'
+    assert 'foo' in md
+    assert 'key' not in md
+    assert md['foo'] == 'value'
+    mdlist[0].value = 'bar'
+    assert mdlist[0].value == 'bar'
+    assert md['foo'] == 'bar'
+
+    # Append new NVPair
+    with pytest.warns(DeprecationWarning) as w:
+        nvpair = GiftiNVPairs('key', 'value')
+    mdlist.append(nvpair)
+    assert len(mdlist) == 2
+    assert mdlist[1].name == 'key'
+    assert mdlist[1].value == 'value'
+    assert len(md) == 2
+    assert md == {'foo': 'bar', 'key': 'value'}
+
+    # Clearing empties both
+    mdlist.clear()
+    assert len(mdlist) == 0
+    assert len(md) == 0
+
+    # Extension adds multiple keys
+    with pytest.warns(DeprecationWarning) as w:
+        foobar = GiftiNVPairs('foo', 'bar')
+    mdlist.extend([nvpair, foobar])
+    assert len(mdlist) == 2
+    assert len(md) == 2
+    assert md == {'key': 'value', 'foo': 'bar'}
+
+    # Insertion updates list order, though we don't attempt to preserve it in the dict
+    with pytest.warns(DeprecationWarning) as w:
+        lastone = GiftiNVPairs('last', 'one')
+    mdlist.insert(1, lastone)
+    assert len(mdlist) == 3
+    assert len(md) == 3
+    assert mdlist[1].name == 'last'
+    assert mdlist[1].value == 'one'
+    assert md == {'key': 'value', 'foo': 'bar', 'last': 'one'}
+
+    # Popping returns a pair
+    mypair = mdlist.pop(0)
+    assert isinstance(mypair, GiftiNVPairs)
+    assert mypair.name == 'key'
+    assert mypair.value == 'value'
+    assert len(mdlist) == 2
+    assert len(md) == 2
+    assert 'key' not in md
+    assert md == {'foo': 'bar', 'last': 'one'}
+    # Modifying the pair now does not affect md
+    mypair.name = 'completelynew'
+    mypair.value = 'strings'
+    assert 'completelynew' not in md
+    assert md == {'foo': 'bar', 'last': 'one'}
+    # Check popping from the end (lastone inserted before foobar)
+    lastpair = mdlist.pop()
+    assert len(mdlist) == 1
+    assert len(md) == 1
+    assert md == {'last': 'one'}
+
+    # And let's remove an old pair with a new object
+    with pytest.warns(DeprecationWarning) as w:
+        lastoneagain = GiftiNVPairs('last', 'one')
+    mdlist.remove(lastoneagain)
+    assert len(mdlist) == 0
+    assert len(md) == 0
+
+
 def test_gifti_label_rgba():
     rgba = np.random.rand(4)
     kwargs = dict(zip(['red', 'green', 'blue', 'alpha'], rgba))

nibabel/gifti/tests/test_parse_gifti_fast.py

@@ -147,7 +147,7 @@ def test_default_types():
         # GiftiMetaData
         assert_default_types(img.meta)
         # GiftiNVPairs - Remove in NIB6
-        with pytest.warns(FutureWarning):
+        with pytest.warns(DeprecationWarning):
             for nvpair in img.meta.data:
                 assert_default_types(nvpair)
         # GiftiLabelTable
@@ -161,7 +161,7 @@ def test_default_types():
             # GiftiMetaData
             assert_default_types(darray.meta)
             # GiftiNVPairs - Remove in NIB6
-            with pytest.warns(FutureWarning):
+            with pytest.warns(DeprecationWarning):
                 for nvpair in darray.meta.data:
                     assert_default_types(nvpair)
 

nibabel/tests/test_removalschedule.py

@@ -12,6 +12,8 @@
 ]
 
 OBJECT_SCHEDULE = [
+    ("7.0.0", [("nibabel.gifti.gifti", "GiftiNVPairs"),
+    ]),
     ("6.0.0", [("nibabel.loadsave", "guessed_image_type"),
                ("nibabel.loadsave", "read_img_data"),
                ("nibabel.orientations", "flip_axis"),
@@ -41,6 +43,7 @@
 ATTRIBUTE_SCHEDULE = [
     ("7.0.0", [("nibabel.gifti.gifti", "GiftiMetaData", "from_dict"),
                ("nibabel.gifti.gifti", "GiftiMetaData", "metadata"),
+               ("nibabel.gifti.gifti", "GiftiMetaData", "data"),
     ]),
     ("5.0.0", [("nibabel.dataobj_images", "DataobjImage", "get_data"),
                ("nibabel.freesurfer.mghformat", "MGHHeader", "_header_data"),