Merge pull request #298 from clamsproject/develop

releasing 1.0.18

Author: keighrim

documentation/plugins.rst

@@ -28,6 +28,7 @@ To add a document location handler plugin, you need to implement a Python `"pack
 
 #. the package must be named ``mmif_docloc_<SCHEME>``. For example, to implement a handler for ``s3`` scheme, the package name must be ``mmif_docloc_s3``. The prefix is important as it's used in the plugin discovery process from the core ``mmif-python`` modules.
 #. the top module of the package must have a function named ``resolve``. The function must take a single argument, which is a :class:`str` of the document location URI. The function must return a :class:`str` of the local file path. For example, if the document location is ``s3://mybucket/myfile.mp4``, a Python user should be able to to something like this; 
+#. Optionally (but highly recommended), the top module also can provide another function named ``help``. The function must take no arguments and return a :class:`str` that explains how the input string to the ``resolve`` function should be formatted.
 
 .. code-block:: python
 
@@ -76,6 +77,11 @@ And the plugin code.
        else:
            raise ValueError(f'cannot handle document location scheme: {docloc}')
 
+   def help():
+       return "location format: `<DOCUMENT_ID>.video`"
+
+
+
 Bulit-in Document Location Scheme Plugins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

mmif/serialize/annotation.py

@@ -323,6 +323,10 @@ def add_property(self, name: str,
            only keep the latest value (in order of appearances in views list) of 
            the property, effectively overwriting the previous values.
         """
+        # we don't checking if this k-v already exists in _original (new props) or _ephemeral (read from existing MMIF)
+        # because it is impossible to keep the _original updated when a new annotation is added (via `new_annotation`)
+        # without look across other views and top-level documents list. Also see 
+        # ``mmif.serialize.mmif.Mmif.generate_capital_annotations`` for where the "de-duplication" happens.
         if name == "text":
             self.properties.text = Text(value)
         elif name == "mime":

mmif/serialize/mmif.py

@@ -115,13 +115,24 @@ def append(self, value: View, overwrite=False) -> None:
         """
         super()._append_with_key(value.id, value, overwrite)
 
-    def get_last(self) -> Optional[View]:
+    def get_last_contentful_view(self) -> Optional[View]:
         """
-        Returns the last view appended to the list.
+        Returns the last view that is contentful, i.e., has no error or warning .
         """
         for view in reversed(self._items.values()):
             if 'error' not in view.metadata and 'warning' not in view.metadata:
                 return view
+    
+    def get_last_view(self) -> Optional[View]:
+        """
+        Returns the last view appended.
+        """
+        if self._items:
+            return self._items[list(self._items.keys())[-1]]
+            
+    def get_last(self) -> Optional[View]:
+        warnings.warn('get_last() is deprecated, use get_last_contentful_view() instead.', DeprecationWarning)
+        return self.get_last_contentful_view()
 
 
 class Mmif(MmifObject):
@@ -278,13 +289,23 @@ def generate_capital_annotations(self):
         See https://github.com/clamsproject/mmif-python/issues/226 for rationale
         behind this behavior and discussion.
         """
-        # this view will be the default kitchen sink for all generated annotations
-        last_view = self.views.get_last()
+        # this view will be the default kitchen sink for all generated annotations.
+        last_view = self.views.get_last_contentful_view()
+        
         # proceed only when there's at least one view
         if last_view:
+            
+            # this app name is used to check a view is generated by the "currently running" app.
+            # knowing the currently running app is important so that properties of `Document` objects generated by the 
+            # current app can be properly recorded inside the `Document` objects (since they are "writable" to the 
+            # current app), instead of being recorded in a separate `Annotation` object.
+            current_app = last_view.metadata.app
+
             # to avoid duplicate property recording, this will be populated with
             # existing Annotation objects from all existing views
             existing_anns = defaultdict(lambda: defaultdict(dict))
+            # ideally, if we can "de-duplicate" props at `add_property()` time, that'd be more efficient, 
+            # but that is impossible without looking for the target `document` across other views and top documents list
 
             # new properties to record in the current serialization call
             anns_to_write = defaultdict(dict)
@@ -296,20 +317,30 @@ def generate_capital_annotations(self):
                     for ann in view.get_annotations(AnnotationTypes.Annotation):
                         if doc_id is None:
                             doc_id = ann.get_property('document')
+                        # only if we are sure that the document ID is unique across all views... (with v_id prefix)
+                        # TODO (krim @ 7/15/24): update id checking once https://github.com/clamsproject/mmif/issues/228 is resolved
+                        if not any([doc_id == doc.id for doc in self.documents]) and Mmif.id_delimiter not in doc_id:
+                            doc_id = f"{view.id}{Mmif.id_delimiter}{doc_id}"
                         existing_anns[doc_id].update(ann.properties)
                 for doc in view.get_documents():
-                    anns_to_write[doc.id].update(doc._props_pending)
+                    anns_to_write[doc.long_id].update(doc._props_pending)
             for doc in self.documents:
-                anns_to_write[doc.id].update(doc._props_pending)
+                anns_to_write[doc.long_id].update(doc._props_pending)
             # additional iteration of views, to find a proper view to add the 
             # generated annotations. If none found, use the last view as the kitchen sink
             last_view_for_docs = defaultdict(lambda: last_view)
             doc_ids = set(anns_to_write.keys())
             for doc_id in doc_ids:
+                if len(last_view.annotations) == 0:
+                    # meaning, this new app didn't generate any annotation except for these document properties
+                    # thus, we should add capital annotations to the last (empty) view
+                    last_view_for_docs[doc_id] = last_view
+                    break
                 for view in reversed(self.views):
                     # first try to find out if this view "contains" any annotation to the doc
                     # then, check for individual annotations
-                    if [cont for cont in view.metadata.contains.values() if cont.get('document', None) == doc_id] \
+                    # TODO (krim @ 7/15/24): update id checking once https://github.com/clamsproject/mmif/issues/228 is resolved
+                    if [cont for cont in view.metadata.contains.values() if doc_id.endswith(cont.get('document', 'TODO:this endswith test is a temporal solution we use until long_id is forced everywhere'))] \
                             or list(view.get_annotations(document=doc_id)):
                         last_view_for_docs[doc_id] = view
                         break
@@ -323,14 +354,18 @@ def generate_capital_annotations(self):
                     if k != 'id' and existing_anns[doc_id][k] != v:
                         props[k] = v
                 if props:
-                    if len(anns_to_write) == 1:
-                        # if there's only one document, we can record the doc_id in the contains metadata
-                        last_view_for_docs[doc_id].metadata.new_contain(AnnotationTypes.Annotation, document=doc_id)
-                        props.pop('document', None)
+                    view_to_write = last_view_for_docs[doc_id]
+                    if view_to_write.metadata.app == current_app and view_to_write.annotations.get(doc_id) is not None:
+                        view_to_write.get_document_by_id(doc_id).properties.update(props)
                     else:
-                        # otherwise, doc_id needs to be recorded in the annotation property
-                        props['document'] = doc_id
-                    last_view_for_docs[doc_id].new_annotation(AnnotationTypes.Annotation, **props)
+                        if len(anns_to_write) == 1:
+                            # if there's only one document, we can record the doc_id in the contains metadata
+                            view_to_write.metadata.new_contain(AnnotationTypes.Annotation, document=doc_id)
+                            props.pop('document', None)
+                        else:
+                            # otherwise, doc_id needs to be recorded in the annotation property
+                            props['document'] = doc_id
+                        view_to_write.new_annotation(AnnotationTypes.Annotation, **props)
 
     def sanitize(self):
         """

mmif/serialize/view.py

@@ -10,6 +10,7 @@
 from typing import Dict, Union, Optional, Generator, List, cast
 
 from mmif import DocumentTypes, AnnotationTypes, ThingTypesBase, ClamsTypesBase
+from mmif.serialize import model
 from mmif.serialize.annotation import Annotation, Document
 from mmif.serialize.model import PRMTV_TYPES, MmifObject, DataList, DataDict
 
@@ -439,6 +440,16 @@ def append(self, value: Union[Annotation, Document], overwrite=False) -> None:
         :return: None
         """
         super()._append_with_key(value.id, value, overwrite)
+        
+    def __getitem__(self, key: str):
+        """
+        specialized getter implementation to workaround https://github.com/clamsproject/mmif/issues/228
+        # TODO (krim @ 7/12/24): annotation ids must be in the long form in the future, so this check will be unnecessary once https://github.com/clamsproject/mmif/issues/228 is resolved. 
+        """
+        if ":" in key:
+            _, aid = key.split(":")
+            return self._items.__getitem__(aid)
+        return self._items.get(key, None)
 
 
 class ContainsDict(DataDict[ThingTypesBase, Contain]):

mmif_docloc_http/__init__.py

@@ -10,3 +10,7 @@ def resolve(docloc):
             raise ValueError(f'cannot handle document location scheme: {docloc}')
     except urllib.error.URLError as e:
         raise e
+    
+    
+def help():
+    return "location must be a URL string."

tests/test_serialize.py

@@ -1118,11 +1118,11 @@ def test_document_added_properties(self):
         # then converted to an `Annotation` annotation during serialization at `Mmif`-level
         mmif.add_document(doc1)
         ## no Annotation before serialization
-        self.assertEqual(0, len(list(mmif.views.get_last().get_annotations(AnnotationTypes.Annotation, author='me'))))
+        self.assertEqual(0, len(list(mmif.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation, author='me'))))
         ## after serialization, the `Annotation` annotation should be added to the last view
         ## note that we didn't add any views, so the last view before and after the serialization are the same
         mmif_roundtrip = Mmif(mmif.serialize())
-        self.assertTrue(next(mmif_roundtrip.views.get_last().get_annotations(AnnotationTypes.Annotation, author='me')))
+        self.assertTrue(next(mmif_roundtrip.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation, author='me')))
         # finally, when deserialized back to a Mmif instance, the `Annotation` props should be added
         # as a property of the document 
         doc1_mmif_roundtrip = mmif_roundtrip.get_document_by_id('doc1')
@@ -1135,7 +1135,7 @@ def test_document_adding_duplicate_properties(self):
         mmif.add_document(doc1)
         did = 'doc1'
         # last view before serialization rounds
-        r0_vid = mmif.views.get_last().id
+        r0_vid = mmif.views.get_last_contentful_view().id
         doc1.at_type = DocumentTypes.TextDocument
         doc1.id = did
         doc1.location = 'aScheme:///data/doc1.txt'
@@ -1165,7 +1165,7 @@ def test_document_adding_duplicate_properties(self):
         self.assertEqual(2, len(doc1._props_pending))
         mmif_roundtrip2 = Mmif(mmif_roundtrip1.serialize())
         ## but not serialized
-        self.assertEqual(0, len(list(mmif_roundtrip2.views.get_last().get_annotations(AnnotationTypes.Annotation))))
+        self.assertEqual(0, len(list(mmif_roundtrip2.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))))
 
         # adding non-duplicate value should be serialized into a new Annotation object
         # even when there is a duplicate key in a previous view
@@ -1211,7 +1211,7 @@ def test_document_added_properties_with_manual_capital_annotation(self):
         doc1.add_property('author', 'me')
         mmif_roundtrip = Mmif(mmif.serialize())
         ## should be only one Annotation object, from manual call of `new_annotation`
-        self.assertEqual(1, len(list(mmif_roundtrip.views.get_last().get_annotations(AnnotationTypes.Annotation))))
+        self.assertEqual(1, len(list(mmif_roundtrip.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))))
 
         # test with duplicate property added by a downstream app
         mmif_roundtrip = Mmif(mmif.serialize())
@@ -1223,9 +1223,9 @@ def test_document_added_properties_with_manual_capital_annotation(self):
         # author=me is already in the input MMIF
         doc1_prime.add_property('author', 'me')
         mmif_roundtrip2 = Mmif(mmif_roundtrip.serialize())
-        self.assertEqual(1, len(list(mmif.views.get_last().get_annotations(AnnotationTypes.Annotation))))
+        self.assertEqual(1, len(list(mmif.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))))
         # none should be added
-        self.assertEqual(0, len(list(mmif_roundtrip2.views.get_last().get_annotations(AnnotationTypes.Annotation))))
+        self.assertEqual(0, len(list(mmif_roundtrip2.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))))
 
         # test with same key, different value
         mmif_roundtrip = Mmif(mmif.serialize())
@@ -1238,13 +1238,13 @@ def test_document_added_properties_with_manual_capital_annotation(self):
         doc1_prime.add_property('author', 'you')
         mmif_roundtrip2 = Mmif(mmif_roundtrip.serialize())
         # the Annotation in the previous view should be preserved
-        self.assertEqual(1, len(list(mmif.views.get_last().get_annotations(AnnotationTypes.Annotation))))
+        self.assertEqual(1, len(list(mmif.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))))
         # but a new one should be added
-        self.assertEqual(1, len(list(mmif_roundtrip2.views.get_last().get_annotations(AnnotationTypes.Annotation))))
+        self.assertEqual(1, len(list(mmif_roundtrip2.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))))
 
-        self.assertEqual('me', list(mmif.views.get_last().get_annotations(AnnotationTypes.Annotation))[0].get_property(
+        self.assertEqual('me', list(mmif.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))[0].get_property(
             'author'))
-        self.assertEqual('you', list(mmif_roundtrip2.views.get_last().get_annotations(AnnotationTypes.Annotation))[
+        self.assertEqual('you', list(mmif_roundtrip2.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))[
             0].get_property('author'))
 
     def test_capital_annotation_generation_viewfinder(self):
@@ -1269,6 +1269,28 @@ 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_capital_annotation_nongeneration_for_writable_documents(self):
+        mmif = Mmif(validate=False)
+        doc = Document()
+        doc.at_type = DocumentTypes.TextDocument
+        doc.id = f'doc0'
+        doc.location = f'aScheme:///data/doc0.txt'
+        mmif.add_document(doc)
+
+        v = mmif.new_view()
+        v.metadata.app = tester_appname
+        vid = v.id
+        new_td_id = mmif[vid].new_textdocument(text='new text', document='doc0', origin='transformation').id
+        doc.add_property('author', 'me')
+        
+        mmif_roundtrip = Mmif(mmif.serialize())
+        
+        self.assertTrue(AnnotationTypes.Annotation in mmif_roundtrip[vid].metadata.contains)
+        self.assertTrue(mmif_roundtrip.get_document_by_id('doc0').get_property('author'), 'me')
+        self.assertTrue(next(mmif_roundtrip[vid].get_annotations(AnnotationTypes.Annotation)).get_property('author'), 'me')
+        self.assertTrue(next(mmif_roundtrip[vid].get_annotations(AnnotationTypes.Annotation)).get_property('document'), doc.id)
+        self.assertTrue(mmif_roundtrip[new_td_id].get_property('origin'), 'transformation')
 
     def test_deserialize_with_whole_mmif(self):
         for i, datum in self.data.items():