diff --git a/.github/workflows/release-check.yml b/.github/workflows/release-check.yml
new file mode 100644
index 00000000..968d1f88
--- /dev/null
+++ b/.github/workflows/release-check.yml
@@ -0,0 +1,11 @@
+name: "🔖 Release PR check"
+
+on:
+ pull_request:
+ branches: [main]
+ types: [opened, synchronize, reopened, edited]
+
+jobs:
+ release-check:
+ name: "🤙 Call SDK release-check workflow"
+ uses: clamsproject/.github/.github/workflows/sdk-release-check.yml@main
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 1153e9af..0dcd3c50 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -13,10 +13,17 @@ We follow a Gitflow-inspired branching model to maintain a stable `main` branch
- Once work is complete, open a PR targeting the `develop` branch.
- **Communication**: High-level discussion and planning should occur in the issue thread. The PR conversation is strictly for code review and implementation-specific feedback.
5. **Releases**:
- - When `develop` is ready for a new release, open a PR from `develop` to `main` using the "release" PR template.
+ - When `develop` is ready for a new release, open a PR from `develop` to `main` using the "release" PR template. See the **Releases** section below for the required title format and preparation steps.
- After merging the release candidate into `main`, manually tag the commit with the version number. This tag triggers the automated CI/CD pipeline for publishing.
6. **Branch Protection**: Both `main` and `develop` are protected branches. Direct pushes are disabled; all changes must be introduced via Pull Requests.
+## Releases
+
+Release PRs (from `develop` to `main`) are gated by the `release-check` CI workflow. Before opening one:
+
+1. Title the PR exactly `releasing X.Y.Z` (strict semver); `release-check` reads the version from the title.
+2. Run `python build-tools/prep_release.py X.Y.Z` and commit its changes. See inside the prep script to see what's actually prepared.
+
## CLI Scripts
The `mmif` command-line interface supports subcommands (e.g., `mmif source`, `mmif describe`). These are implemented as Python modules in `mmif/utils/cli/`.
diff --git a/build-tools/prep_release.py b/build-tools/prep_release.py
new file mode 100644
index 00000000..82d673b1
--- /dev/null
+++ b/build-tools/prep_release.py
@@ -0,0 +1,91 @@
+"""
+Prepare release artifacts for the mmif-python package.
+
+Run while preparing a release PR, passing the version being released::
+
+ python build-tools/prep_release.py X.Y.Z
+
+Currently this adds a row to ``documentation/target-versions.csv`` for
+the release version, resolving the target MMIF spec version from the
+installed package metadata (``mmif.__specver__``). This script is the
+single place that writes the target-versions table; the docs build only
+renders it, and CI verifies (on the release PR) that a row for the PR's
+version exists.
+
+Requires an editable install (``pip install -e .``) so that
+``mmif.__specver__`` is importable.
+"""
+import argparse
+import re
+import sys
+from pathlib import Path
+
+PROJECT_ROOT = Path(__file__).parent.parent
+CSV_PATH = PROJECT_ROOT / 'documentation' / 'target-versions.csv'
+
+VERSION_RE = re.compile(r'^\d+\.\d+\.\d+$')
+TOP_ROW_RE = re.compile(r'`(\d+\.\d+\.\d+) <')
+
+
+def _row(version, specver):
+ return (
+ f'`{version} `__,'
+ f'`{specver} `__'
+ )
+
+
+def _version_tuple(v):
+ return tuple(int(x) for x in v.split('.'))
+
+
+def _top_version(data_lines):
+ if not data_lines:
+ return None
+ m = TOP_ROW_RE.match(data_lines[0])
+ return m.group(1) if m else None
+
+
+def main():
+ parser = argparse.ArgumentParser(
+ description="Prepare release artifacts for mmif-python."
+ )
+ parser.add_argument(
+ "version", metavar="X.Y.Z",
+ help="the release version being prepared",
+ )
+ args = parser.parse_args()
+ version = args.version
+
+ if not VERSION_RE.match(version):
+ sys.exit(f"Error: '{version}' is not a valid X.Y.Z version.")
+
+ try:
+ import mmif
+ specver = mmif.__specver__
+ except (ImportError, AttributeError):
+ sys.exit("Error: cannot read mmif.__specver__. "
+ "Run `pip install -e .` first.")
+
+ lines = CSV_PATH.read_text().splitlines()
+ header, data = lines[0], lines[1:]
+ top = _top_version(data)
+
+ if top is not None and _version_tuple(version) < _version_tuple(top):
+ sys.exit(f"Error: {version} is older than the current top row "
+ f"{top}; refusing to insert an out-of-order version.")
+
+ new_row = _row(version, specver)
+ if top == version:
+ data[0] = new_row # update in place (e.g. spec changed)
+ action = "updated"
+ else:
+ data.insert(0, new_row) # prepend as the new latest
+ action = "added"
+
+ CSV_PATH.write_text('\n'.join([header] + data) + '\n')
+ print(f"target-versions.csv: {action} row "
+ f"{version} -> MMIF spec {specver}")
+
+
+if __name__ == "__main__":
+ main()
diff --git a/documentation/conf.py b/documentation/conf.py
index 8313b1ca..2b57e9f9 100644
--- a/documentation/conf.py
+++ b/documentation/conf.py
@@ -6,7 +6,6 @@
import inspect
import json
import os
-import re
import subprocess
import sys
import textwrap
@@ -148,42 +147,6 @@ def linkcode_resolve(domain, info):
return None
-def update_target_versions(app):
- """
- Update documentation/target-versions.csv with the current version and spec version.
- This replaces the old logic in setup.py.
- """
- # Check if we have __specver__ (it might not be there if not installed via setup.py develop)
- if not hasattr(mmif, '__specver__'):
- return
-
- current_ver = mmif.__version__
- # Skip dev/dummy versions to avoid dirtying the git-tracked CSV
- if 'dev' in current_ver or not re.match(r'^\d+\.\d+\.\d+$', current_ver):
- return
- spec_ver = mmif.__specver__
-
- csv_path = proj_root_dir / 'documentation' / 'target-versions.csv'
- if not csv_path.exists():
- return
-
- # Read existing content
- with open(csv_path, 'r') as f:
- lines = f.readlines()
-
- # Check if current version is already in the file (first data line)
- # lines[0] is header, lines[1] is latest version
- if len(lines) > 1 and lines[1].startswith(f'{current_ver},'):
- return
-
- # Insert new version
- logger.info(f"Updating target-versions.csv: {current_ver} -> {spec_ver}")
- lines.insert(1, f'{current_ver},"{spec_ver}"\n')
-
- with open(csv_path, 'w') as f:
- f.writelines(lines)
-
-
def generate_cli_rst(app):
from mmif import prep_argparser_and_subcmds
@@ -292,7 +255,6 @@ def run_apidoc(app):
def setup(app):
try:
app.connect('builder-inited', run_apidoc)
- app.connect('builder-inited', update_target_versions)
app.connect('builder-inited', generate_cli_rst)
app.connect('builder-inited', generate_whatsnew_rst)
except ImportError:
diff --git a/documentation/target-versions.csv b/documentation/target-versions.csv
index 933e4bf3..3365fa14 100644
--- a/documentation/target-versions.csv
+++ b/documentation/target-versions.csv
@@ -1,47 +1,52 @@
"``mmif-python`` version","Target MMIF Specification"
-1.3.0,"1.1.0"
-1.2.0,"1.1.0"
-1.1.2,"1.1.0"
-1.1.1,"1.1.0"
-1.1.0,"1.1.0"
-1.0.19,"1.0.5"
-1.0.18,"1.0.5"
-1.0.17,"1.0.5"
-1.0.16,"1.0.4"
-1.0.15,"1.0.4"
-1.0.14,"1.0.4"
-1.0.13,"1.0.4"
-1.0.12,"1.0.4"
-1.0.11,"1.0.4"
-1.0.10,"1.0.2"
-1.0.9,"1.1.0"
-1.0.8,"1.0.0"
-1.0.7,"1.0.0"
-1.0.6,"1.0.0"
-1.0.5,"1.0.0"
-1.0.4,"1.0.0"
-1.0.3,"1.0.0"
-1.0.2,"1.0.0"
-1.0.1,"1.0.0"
-1.0.0,"0.5.0"
-0.5.2,"0.5.0"
-0.5.1,"0.5.0"
-0.5.0,"0.5.0"
-0.4.8,"0.4.2"
-0.4.7,"0.4.1"
-0.4.6,"0.4.0"
-0.4.5,"0.4.0"
-0.4.4,"0.4.0"
-0.4.3,"0.4.0"
-0.4.2,"0.4.0"
-0.4.1,"0.4.0"
-0.4.0,"0.4.0"
-0.3.5,"0.3.1"
-0.3.4,"0.3.1"
-0.3.3,"0.3.1"
-0.3.2,"0.3.1"
-0.3.1,"0.3.0"
-0.3.0,"0.3.0"
-0.2.2,"0.2.1"
-0.2.1,"0.2.1"
-0.2.0,"0.2.1"
+`1.5.1 `__,`1.2.0 `__
+`1.5.0 `__,`1.2.0 `__
+`1.4.0 `__,`1.1.1 `__
+`1.3.1 `__,`1.1.0 `__
+`1.3.0 `__,`1.1.0 `__
+`1.2.1 `__,`1.1.0 `__
+`1.2.0 `__,`1.1.0 `__
+`1.1.2 `__,`1.1.0 `__
+`1.1.1 `__,`1.1.0 `__
+`1.1.0 `__,`1.1.0 `__
+`1.0.19 `__,`1.0.5 `__
+`1.0.18 `__,`1.0.5 `__
+`1.0.17 `__,`1.0.5 `__
+`1.0.16 `__,`1.0.4 `__
+`1.0.15 `__,`1.0.4 `__
+`1.0.14 `__,`1.0.4 `__
+`1.0.13 `__,`1.0.4 `__
+`1.0.12 `__,`1.0.4 `__
+`1.0.11 `__,`1.0.4 `__
+`1.0.10 `__,`1.0.2 `__
+`1.0.9 `__,`1.1.0 `__
+`1.0.8 `__,`1.0.0 `__
+`1.0.7 `__,`1.0.0 `__
+`1.0.6 `__,`1.0.0 `__
+`1.0.5 `__,`1.0.0 `__
+`1.0.4 `__,`1.0.0 `__
+`1.0.3 `__,`1.0.0 `__
+`1.0.2 `__,`1.0.0 `__
+`1.0.1 `__,`1.0.0 `__
+`1.0.0 `__,`0.5.0 `__
+`0.5.2 `__,`0.5.0 `__
+`0.5.1 `__,`0.5.0 `__
+`0.5.0 `__,`0.5.0 `__
+`0.4.8 `__,`0.4.2 `__
+`0.4.7 `__,`0.4.1 `__
+`0.4.6 `__,`0.4.0 `__
+`0.4.5 `__,`0.4.0 `__
+`0.4.4 `__,`0.4.0 `__
+`0.4.3 `__,`0.4.0 `__
+`0.4.2 `__,`0.4.0 `__
+`0.4.1 `__,`0.4.0 `__
+`0.4.0 `__,`0.4.0 `__
+`0.3.5 `__,`0.3.1 `__
+`0.3.4 `__,`0.3.1 `__
+`0.3.3 `__,`0.3.1 `__
+`0.3.2 `__,`0.3.1 `__
+`0.3.1 `__,`0.3.0 `__
+`0.3.0 `__,`0.3.0 `__
+`0.2.2 `__,`0.2.1 `__
+`0.2.1 `__,`0.2.1 `__
+`0.2.0 `__,`0.2.1 `__
diff --git a/mmif/serialize/annotation.py b/mmif/serialize/annotation.py
index e910fd68..1e5e905b 100644
--- a/mmif/serialize/annotation.py
+++ b/mmif/serialize/annotation.py
@@ -414,22 +414,56 @@ def add_property(self, name: str,
else:
super().add_property(name, value)
- def get(self, prop_name, default=None):
+ def __getitem__(self, prop_name: str):
"""
- Safe property access with optional default value for Document objects.
+ Property access for ``Document`` objects, following the document
+ property hierarchy (highest to lowest priority):
- Searches for a document property by name and returns its value, or a
- default value if not found. Documents have a more complex property
- hierarchy than regular annotations:
+ #. Special fields (``id``, ``location``)
+ #. Pending properties (added via :meth:`add_property` after creation,
+ to be serialized as capital ``Annotation`` objects)
+ #. Original properties (in ``document.properties``)
+ #. Ephemeral properties (from existing ``Annotation`` objects or view
+ ``contains`` metadata)
- Priority order (highest to lowest):
- 1. Special fields ('id', 'location')
- 2. Pending properties (added after creation, to be serialized as ``Annotation`` objects)
- 3. Ephemeral properties (from existing ``Annotation`` annotations or view metadata)
- 4. Original properties (in ``document.properties``)
+ ``Document`` overrides ``__getitem__`` (not only :meth:`get`) so that
+ the ``in`` operator, subscripting, and :meth:`get` all agree.
+ ``__contains__`` is implemented on top of ``__getitem__``, so without
+ this override a property added via :meth:`add_property` (which lands in
+ ``_props_pending``) is readable through :meth:`get` yet invisible to
+ ``in`` until a serialize/deserialize round trip. See issue #390.
- This allows convenient access to all document properties regardless of
- where they're stored internally.
+ :param prop_name: the name of the property to retrieve
+ :raises KeyError: if the property is not found in any of the stores
+ """
+ if prop_name == 'id':
+ # because all three dicts have `id` key as required field, we need
+ # this special case to return the correct value from the correct dict
+ return self.id
+ elif prop_name == 'location':
+ # location is internally stored as `location_`, so the regular
+ # lookup below never finds it. Treat an unset location as absent
+ # (raise) rather than returning None, so the inherited
+ # `__contains__` does not report `'location'` as always present.
+ if self.location is None:
+ raise KeyError(prop_name)
+ return self.location
+ elif prop_name in self._props_pending:
+ return self._props_pending[prop_name]
+ else:
+ # Delegates to Annotation.__getitem__(), which checks
+ # self.properties (_props_original) before _props_ephemeral.
+ # This gives annotation-level properties precedence over
+ # view-level contains defaults (MMIF spec >= 1.1.1).
+ return super().__getitem__(prop_name)
+
+ def get(self, prop_name, default=None):
+ """
+ Safe property access with optional default value for Document objects.
+
+ Searches for a document property by name and returns its value, or
+ ``default`` if not found, following the priority order documented on
+ :meth:`__getitem__`.
:param prop_name: The name of the property to retrieve
:param default: The value to return if the property is not found (default: None)
@@ -455,22 +489,10 @@ def get(self, prop_name, default=None):
add_property : Add a new property to the document
mmif.serialize.mmif.Mmif.generate_capital_annotations : How pending properties are serialized
"""
- if prop_name == 'id':
- # because all three dicts have `id` key as required field, we need
- # this special case to return the correct value from the correct dict
- return self.id
- elif prop_name == 'location':
- # because location is internally stored in self.location_,
- # it doesn't work with regular __getitem__ method
- return self.location
- elif prop_name in self._props_pending:
- return self._props_pending[prop_name]
- else:
- # Delegates to Annotation.__getitem__(), which checks
- # self.properties (_props_original) before _props_ephemeral.
- # This gives annotation-level properties precedence over
- # view-level contains defaults (MMIF spec >= 1.1.1).
- return super().get(prop_name, default)
+ try:
+ return self.__getitem__(prop_name)
+ except KeyError:
+ return default
get_property = get
diff --git a/mmif/utils/video_document_helper.py b/mmif/utils/video_document_helper.py
index 87c1823b..bb1f5b1a 100644
--- a/mmif/utils/video_document_helper.py
+++ b/mmif/utils/video_document_helper.py
@@ -96,20 +96,36 @@ def open_container(video_document: Document):
container = av.open(video_document.location_path(nonexist_ok=False))
stream = container.streams.video[0]
time_base = float(stream.time_base)
- fps = round(float(stream.average_rate), 2)
+ # Frame rate: prefer ffmpeg's guessed rate (av_guess_frame_rate), which
+ # falls back to the stream's base rate (r_frame_rate) when the average
+ # rate (avg_frame_rate) is unset. Matroska/webm commonly leave
+ # `average_rate` as None (which would crash float()) or 0, so it cannot be
+ # used directly. `rate` stays a Fraction for exact frame-count math; both
+ # None and a zero rate are falsy, so the chain yields the first usable one.
+ rate = stream.guessed_rate or stream.average_rate or stream.base_rate
+ fps = round(float(rate), 2) if rate else 0.0
+ # Resolve duration (in seconds) from the most reliable source available.
+ # `stream.duration` is exact for CFR H.264/MP4, but Matroska/webm leave it
+ # unset and carry the length only at the container level
+ # (`container.duration`, in `av.time_base` microseconds).
+ if stream.duration is not None:
+ duration_s = float(stream.duration) * time_base
+ elif container.duration is not None:
+ duration_s = container.duration / av.time_base
+ else:
+ duration_s = None
# `stream.frames` comes from the container header. Verified exact on
# CFR H.264/MP4 inputs even with non-zero start offset; for VFR or
# headerless streams it may be 0, in which case `duration * rate` is
# the best available (approximate) estimate.
if stream.frames > 0:
frame_count = stream.frames
- elif stream.duration is not None and stream.average_rate is not None:
- frame_count = int(round(float(stream.duration) * time_base
- * float(stream.average_rate)))
+ elif duration_s is not None and rate:
+ frame_count = int(round(duration_s * float(rate)))
else:
frame_count = 0
- if stream.duration is not None:
- duration_ms = int(round(float(stream.duration) * time_base * 1000))
+ if duration_s is not None:
+ duration_ms = int(round(duration_s * 1000))
elif frame_count > 0 and fps > 0:
duration_ms = int(round(frame_count / fps * 1000))
else:
diff --git a/tests/black-2997fps.webm b/tests/black-2997fps.webm
new file mode 100644
index 00000000..73e19029
Binary files /dev/null and b/tests/black-2997fps.webm differ
diff --git a/tests/test_serialize.py b/tests/test_serialize.py
index 78d4ed9a..f2d36e68 100644
--- a/tests/test_serialize.py
+++ b/tests/test_serialize.py
@@ -1304,7 +1304,36 @@ def test_document_added_properties(self):
doc1_mmif_roundtrip = mmif_roundtrip['doc1']
self.assertEqual(0, len(doc1_mmif_roundtrip._props_pending))
self.assertEqual('me', doc1_mmif_roundtrip.get_property('author'))
-
+
+ def test_document_pending_property_membership(self):
+ # regression for issue #390: a property added via `add_property`
+ # lands in `_props_pending` and must be visible through `get_property`,
+ # subscripting, AND the `in` operator consistently -- without needing a
+ # serialize/deserialize round trip first.
+ doc = Document()
+ doc.at_type = DocumentTypes.VideoDocument
+ doc.id = 'doc1'
+ doc.add_property('fps', 29.97)
+ self.assertIn('fps', doc._props_pending)
+ # all three access paths must agree
+ self.assertEqual(29.97, doc.get_property('fps'))
+ self.assertEqual(29.97, doc['fps'])
+ self.assertIn('fps', doc)
+ # special fields and absent keys are unaffected
+ self.assertIn('id', doc)
+ self.assertEqual('doc1', doc['id'])
+ self.assertNotIn('nope', doc)
+ with self.assertRaises(KeyError):
+ _ = doc['nope']
+ # an unset `location` must read as absent, not as always-present None
+ self.assertNotIn('location', doc)
+ with self.assertRaises(KeyError):
+ _ = doc['location']
+ self.assertIsNone(doc.get('location'))
+ doc.location = 'file:///tmp/doc1.mp4'
+ self.assertIn('location', doc)
+ self.assertEqual('file:///tmp/doc1.mp4', doc['location'])
+
def test_document_adding_duplicate_properties(self):
mmif = Mmif(self.data['everything']['string'])
doc1 = Document()
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 188a0774..2bae378f 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -183,6 +183,26 @@ def test_open_container(self):
finally:
c.close()
+ def test_open_container_matroska(self):
+ # regression for issue #390: Matroska/webm leave the per-stream
+ # `frames`/`duration` fields unset, so open_container must fall back to
+ # the container-level duration instead of caching frameCount/duration 0.
+ vd = Document({
+ "@type": "http://mmif.clams.ai/vocabulary/VideoDocument/v1",
+ "properties": {
+ "mime": "video",
+ "id": "o1",
+ "location": f"file://{pathlib.Path(__file__).parent}/black-2997fps.webm",
+ }
+ })
+ c = vdh.open_container(vd)
+ try:
+ self.assertAlmostEqual(29.97, vd.get_property('fps'), places=1)
+ self.assertGreater(vd.get_property('frameCount'), 0)
+ self.assertGreater(vd.get_property('duration'), 0)
+ finally:
+ c.close()
+
def test_sample_timepoints(self):
# half-open interval; step in ms
self.assertEqual([0, 100, 200, 300, 400],