fixed empty annotation props weren't get-able (#274)

Author: keighrim

mmif/serialize/annotation.py

@@ -415,26 +415,31 @@ def __delitem__(self, key: str) -> None:
 
     def __iter__(self) -> Iterator[str]:
         """
-        ``__iter__`` on Mapping should basically work as ``keys()`` method of vanilla dict
-        however, when MMIF objects are serialized, all optional (not in ``_req_atts``),
-        empty props are ignored (note that emtpy but required props are serialized 
-        with the *emtpy* value). 
-        Hence, this ``__iter__`` method should also work in the same way and 
-        ignore empty optional props. 
+        ``__iter__`` on Mapping should basically work as ``keys()`` method 
+        of vanilla dict.
         """
         for key in itertools.chain(self._named_attributes(), self._unnamed_attributes):
-            if key in self._required_attributes:
-                yield key
-            else:
-                try:
-                    self.__getitem__(key)
-                    yield key
-                except KeyError:
-                    pass
+            yield key
+                
+    def __getitem__(self, key):
+        """
+        Parent MmifObject class has a __getitem__ method that checks if 
+        the value is empty when asked for an unnamed attribute. But for 
+        AnnotationProperties, any arbitrary property that's added 
+        explicitly by the user (developer) should not be ignored and 
+        returned even the value is empty. 
+        """
+        if key in self._named_attributes():
+            return self.__dict__[key]
+        else:
+            return self._unnamed_attributes[key]
 
     def __init__(self, mmif_obj: Optional[Union[bytes, str, dict]] = None) -> None:
         self.id: str = ''
+        # any individual at_type (subclassing this class) can have its own set of required attributes
         self._required_attributes = ["id"]
+        # allowing additional attributes for arbitrary annotation properties
+        self._unnamed_attributes = {}
         super().__init__(mmif_obj)
 
 

mmif/serialize/model.py

@@ -44,35 +44,39 @@ class MmifObject(object):
 
     1. _unnamed_attributes:
        Only can be either None or an empty dictionary. If it's set to None,
-       it means the class won't take any ``Additional Attributes`` in the JSON
-       schema sense. If it's a dict, users can throw any k-v pairs to the
-       class, EXCEPT for the reserved two key names.
+       it means the class won't take any ``Additional Attributes`` in the 
+       JSON schema sense. If it's an empty dict, users can throw any k-v 
+       pairs to the class, as long as the key is not a "reserved" name, 
+       and those additional attributes will be stored in this dict while
+       in memory. 
     2. _attribute_classes:
        This is a dict from a key name to a specific python class to use for
        deserialize the value. Note that a key name in this dict does NOT
        have to be a *named* attribute, but is recommended to be one.
     3. _required_attributes:
-       This is a simple list of names of attributes that are required in the object.
-       When serialize, an object will skip its *empty* (e.g. zero-length, or None)
-       attributes unless they are in this list. Otherwise, the serialized JSON
-       string would have empty representations (e.g. ``""``, ``[]``).
+       This is a simple list of names of attributes that are required in 
+       the object. When serialize, an object will skip its *empty* (e.g. 
+       zero-length, or None) attributes unless they are in this list. 
+       Otherwise, the serialized JSON string would have empty 
+       representations (e.g. ``""``, ``[]``).
     4. _exclude_from_diff:
-       This is a simple list of names of attributes that should be excluded from
-       the diff calculation in ``__eq__``. 
+       This is a simple list of names of attributes that should be excluded 
+       from the diff calculation in ``__eq__``. 
 
     # TODO (krim @ 8/17/20): this dict is however, a duplicate with the type hints in the class definition.
-    Maybe there is a better way to utilize type hints (e.g. getting them as a programmatically), but for now
-    developers should be careful to add types to hints as well as to this dict.
+    Maybe there is a better way to utilize type hints (e.g. getting them 
+    as a programmatically), but for now developers should be careful to 
+    add types to hints as well as to this dict.
 
     Also note that those special attributes MUST be set in the __init__()
     before calling super method, otherwise deserialization will not work.
 
     And also, a subclass that has one or more *named* attributes, it must
-    set those attributes in the __init__() before calling super method. When
-    serializing a MmifObject, all *empty* attributes will be ignored, so for
-    optional named attributes, you must leave the values empty (len == 0), but
-    NOT None. Any None-valued named attributes will cause issues with current
-    implementation.
+    set those attributes in the __init__() before calling super method. 
+    When serializing a MmifObject, all *empty* attributes will be ignored, 
+    so for optional named attributes, you must leave the values empty 
+    (len == 0), but NOT None. Any None-valued named attributes will cause 
+    issues with current implementation.
 
     :param mmif_obj: JSON string or `dict` to initialize an object.
      If not given, an empty object will be initialized, sometimes with
@@ -272,8 +276,8 @@ def __len__(self) -> int:
         """
         Returns number of attributes that are not *empty*. 
         """
-        return sum([named in self and not self.is_empty(self[named]) for named in self._named_attributes()]) \
-               + (len(self._unnamed_attributes) if self._unnamed_attributes else 0)
+        return (sum([named in self and not self.is_empty(self[named]) for named in self._named_attributes()]) 
+                + (len(self._unnamed_attributes) if self._unnamed_attributes else 0))
 
     def __setitem__(self, key, value) -> None:
         if key in self.reserved_names:

tests/test_serialize.py

@@ -15,7 +15,7 @@
 from mmif.serialize import *
 from mmif.serialize.model import *
 from mmif.serialize.view import ContainsDict, ErrorDict
-from mmif.vocabulary import AnnotationTypes, DocumentTypes
+from mmif.vocabulary import AnnotationTypes, DocumentTypes, ThingType
 from tests.mmif_examples import *
 
 # Flags for skipping tests
@@ -795,7 +795,29 @@ def test_annotation_properties(self):
         for k, v in ann_obj.properties.items():
             self.assertTrue(k is not None)
             self.assertTrue(v is not None)
-    
+            
+    def test_empty_annotation_property(self):
+        a = Annotation({
+            '@type': ThingType.Thing,
+            'properties': {
+                'id': 'a1',
+                'empty_str_prop': "",
+                'nonempty_str_prop': "string",
+                'empty_lst_prop': []
+            }
+        })
+        self.assertEqual(a['empty_str_prop'], "")
+        self.assertEqual(a['empty_lst_prop'], [])
+        self.assertEqual(4, len(a.properties.keys()))
+        a_serialized = a.serialize()
+        json.loads(a_serialized)
+        self.assertEqual(json.loads(a_serialized)['properties']['empty_str_prop'], "")
+        self.assertEqual(json.loads(a_serialized)['properties']['empty_lst_prop'], [])
+        a_roundtrip = Annotation(a_serialized)
+        self.assertEqual(a_roundtrip.get_property('empty_str_prop'), "")
+        self.assertEqual(a_roundtrip.get_property('empty_lst_prop'), [])
+        self.assertEqual(4, len(a.properties.keys()))
+
     def test_annotation_ephemeral_properties(self):
         mmif = self.data['everything']['mmif']
         first_view_first_ann = mmif['v1']['s1']
@@ -1116,8 +1138,6 @@ def test_capital_annotation_generation_viewfinder(self):
             cap_anns = list(mmif_roundtrip.views[f'v{i}'].get_annotations(AnnotationTypes.Annotation))
             self.assertEqual(1, len(cap_anns))
             self.assertEqual(authors[i-1], cap_anns[0].get_property('author'))
-        
-
 
     def test_deserialize_with_whole_mmif(self):
         for i, datum in self.data.items():