Merge branch 'develop' into 281-improved-rep-frame-extr

Author: keighrim

documentation/autodoc/mmif.serialize.rst

@@ -1,31 +1,33 @@
-mmif.serialize module
-=====================
+mmif.serialize package
+======================
 
-mmif.serialize.model module
+Core package to provide serialization and deserialization of MMIF format. 
+
+``model`` module
 ---------------------------
 
 .. automodule:: mmif.serialize.model
    :members:
    :undoc-members:
    :show-inheritance:
 
-mmif.serialize.mmif module
+``mmif`` module
 --------------------------
 
 .. automodule:: mmif.serialize.mmif
    :members:
    :undoc-members:
    :show-inheritance:
 
-mmif.serialize.view module
+``view`` module
 --------------------------
 
 .. automodule:: mmif.serialize.view
    :members:
    :undoc-members:
    :show-inheritance:
 
-mmif.serialize.annotation module
+``annotation`` module
 --------------------------------
 
 .. automodule:: mmif.serialize.annotation

documentation/autodoc/mmif.utils.rst

@@ -1,18 +1,39 @@
-mmif.utils module
-=================
+mmif.utils package
+==================
 
-mmif.utils.video_document_helper module
+Package containing utility modules for handling different types of source 
+documents, and general implementation of common data structures and 
+algorithms.
+
+``video_document_helper`` module
 ----------------------------------------
 
 .. automodule:: mmif.utils.video_document_helper
    :members:
    :undoc-members:
    :show-inheritance:
 
-mmif.utils.sequence_helper module
+``text_document_helper`` module
+---------------------------------
+
+.. automodule:: mmif.utils.sequence_helper
+   :members:
+   :undoc-members:
+   :show-inheritance:
+
+``sequence_helper`` module
 ---------------------------------
 
 .. automodule:: mmif.utils.sequence_helper
    :members:
    :undoc-members:
    :show-inheritance:
+
+``sequence_helper`` module
+---------------------------------
+
+.. automodule:: mmif.utils.sequence_helper
+   :members:
+   :undoc-members:
+   :show-inheritance:
+

documentation/autodoc/mmif.vocabulary.rst

@@ -1,18 +1,28 @@
 mmif.vocabulary module
 ======================
 
-mmif.vocabulary.annotation\_types module
-----------------------------------------
+Module contains Enum-like classes for CLAMS vocabulary.
 
-.. automodule:: mmif.vocabulary.annotation_types
+.. autoclass:: mmif.vocabulary.ThingTypesBase
+   :show-inheritance:
+.. autoclass:: mmif.vocabulary.ThingType
    :members:
    :undoc-members:
    :show-inheritance:
 
-mmif.vocabulary.document\_types module
---------------------------------------
+.. autoclass:: mmif.vocabulary.ClamsTypesBase
+   :show-inheritance:
+.. autoclass:: mmif.vocabulary.AnnotationTypesBase
+   :show-inheritance:
+.. autoclass:: mmif.vocabulary.DocumentTypesBase
+   :show-inheritance:
+
+.. autoclass:: mmif.vocabulary.AnnotationTypes
+   :members:
+   :undoc-members:
+   :show-inheritance:
 
-.. automodule:: mmif.vocabulary.document_types
+.. autoclass:: mmif.vocabulary.DocumentTypes
    :members:
    :undoc-members:
    :show-inheritance:

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:
+            if 'error' not in view.metadata and 'warnings' 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/utils/__init__.py

@@ -1 +0,0 @@
-from mmif.utils import video_document_helper

mmif/utils/video_document_helper.py

@@ -84,18 +84,19 @@ def extract_frames_as_images(video_document: Document, framenums: List[int], as_
     video = capture(video_document)
     cur_f = 0
     tot_fcount = video_document.get_property(FRAMECOUNT_DOCPROP_KEY)
+    framenums_copy = framenums.copy()
     while True:
-        if not framenums or cur_f > tot_fcount:
+        if not framenums_copy or cur_f > tot_fcount:
             break
         ret, frame = video.read()
-        if cur_f == framenums[0]:
+        if cur_f == framenums_copy[0]:
             if not ret:
                 sec = convert(cur_f, 'f', 's', video_document.get_property(FPS_DOCPROP_KEY))
                 warnings.warn(f'Frame #{cur_f} ({sec}s) could not be read from the video {video_document.id}.')
                 cur_f += 1
                 continue
             frames.append(Image.fromarray(frame[:, :, ::-1]) if as_PIL else frame)
-            framenums.pop(0)
+            framenums_copy.pop(0)
         cur_f += 1
     return frames
 

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."