diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0dcd3c50..f6704270 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -162,7 +162,7 @@ To build documentation for a specific historical version (e.g., `v1.0.0`): python3 build-tools/docs.py --build-ver v1.0.0 ``` -This runs the build in a sandboxed temporary directory. The output will be in `docs-test/`. +This runs the build in a sandboxed temporary directory. A single `--build-ver` places the built docs **flat** in the output directory (`docs-test/` by default). The batch `--build-all-since` mode instead nests each version under a `/` subdirectory of the output directory, so multiple versions can coexist. The flat single-version layout is what the CI docs-publish workflow relies on — it invokes `docs.py --build-ver --output-dir _docs` and uploads `_docs/` as-is, without needing to know whether the project is versioned. > [!NOTE] > In CI, documentation is built and published automatically by the `publish.yml` workflow via the shared `sdk-docs.yml`. The CI calls `docs.py --build-ver --output-dir _docs`. All CLAMS SDK repos use the same `docs.py` CLI interface (`--build-ver`, `--output-dir`). diff --git a/build-tools/docs.py b/build-tools/docs.py index 7f15d390..056e00fb 100644 --- a/build-tools/docs.py +++ b/build-tools/docs.py @@ -188,7 +188,8 @@ def linkcode_resolve(domain, info): return docs_build_dir -def build_docs_for_version(version: str, output_base_dir: Path, repo_path: Path = None): +def build_docs_for_version(version: str, output_base_dir: Path, repo_path: Path = None, + versioned_subdir: bool = True): """ Builds docs for a specific version in a sandbox. Uses local repo copy instead of cloning for efficiency. @@ -197,6 +198,12 @@ def build_docs_for_version(version: str, output_base_dir: Path, repo_path: Path version: Git tag/ref to build output_base_dir: Directory to output built docs repo_path: Path to local repo to copy from (defaults to cwd) + versioned_subdir: when True (batch/from-scratch rebuild), the built + docs are placed under ``output_base_dir//`` so multiple + versions can coexist. When False (single build, as used by CI), + the docs are placed flat into ``output_base_dir`` so the shared + docs-publish workflow can upload them as-is without knowing about + versioning. """ print(f"--- Running in Version Build Mode for version: {version} ---") @@ -226,13 +233,19 @@ def build_docs_for_version(version: str, output_base_dir: Path, repo_path: Path built_docs_path = build_versioned_docs(env, source_path, version) print(f"\n--- Step 4: Copying built artifacts to output directory ---") - version_output_dir = output_base_dir / version - if version_output_dir.exists(): - print(f"Removing existing directory: {version_output_dir}") - shutil.rmtree(version_output_dir) - shutil.copytree(built_docs_path, version_output_dir) + if versioned_subdir: + dest = output_base_dir / version + if dest.exists(): + print(f"Removing existing directory: {dest}") + shutil.rmtree(dest) + shutil.copytree(built_docs_path, dest) + else: + # flat: place the built docs directly into output_base_dir + output_base_dir.mkdir(parents=True, exist_ok=True) + shutil.copytree(built_docs_path, output_base_dir, dirs_exist_ok=True) + dest = output_base_dir - print(f"\nDocumentation for {version} built successfully in: {version_output_dir}") + print(f"\nDocumentation for {version} built successfully in: {dest}") def get_all_tags(): @@ -317,7 +330,9 @@ def main(): for version, error in failed: print(f" {version}: {error}") elif args.build_ver: - build_docs_for_version(args.build_ver, output_dir) + # single build (CI + local single-version): flat output so the + # shared docs-publish workflow uploads it version-agnostically + build_docs_for_version(args.build_ver, output_dir, versioned_subdir=False) else: build_docs_local(Path.cwd(), output_dir) diff --git a/documentation/target-versions.csv b/documentation/target-versions.csv index 3365fa14..d6d11880 100644 --- a/documentation/target-versions.csv +++ b/documentation/target-versions.csv @@ -1,4 +1,5 @@ "``mmif-python`` version","Target MMIF Specification" +`1.5.2 `__,`1.2.0 `__ `1.5.1 `__,`1.2.0 `__ `1.5.0 `__,`1.2.0 `__ `1.4.0 `__,`1.1.1 `__ diff --git a/mmif/serialize/view.py b/mmif/serialize/view.py index f0f057bf..6a99fa49 100644 --- a/mmif/serialize/view.py +++ b/mmif/serialize/view.py @@ -82,6 +82,25 @@ def __init__(self, view_obj: Optional[Union[bytes, str, dict]] = None, parent_mm self._required_attributes = ["id", "metadata", "annotations"] super().__init__(view_obj) self._fix_old_short_ids() + self._rebuild_id_counts() + + def _rebuild_id_counts(self): + """ + Re-seed the per-view auto-ID counters from the annotations loaded during + deserialization. + + ``_id_counts`` is not serialized, so a view read back from JSON starts + with an empty counter. Without this, the next :meth:`new_annotation` + regenerates an ID (e.g. ``an_1``) that already exists in the + deserialized view and :meth:`AnnotationsList.append` raises ``KeyError``. + See issue #393. + """ + for annotation in self.annotations: + local_id = annotation.id.split(self.id_delimiter)[-1] + prefix, _, num = local_id.rpartition('_') + if prefix and num.isdigit(): + self._id_counts[prefix] = max( + self._id_counts.get(prefix, 0), int(num)) def _fix_old_short_ids(self): """ diff --git a/tests/test_serialize.py b/tests/test_serialize.py index f2d36e68..271cf9c1 100644 --- a/tests/test_serialize.py +++ b/tests/test_serialize.py @@ -1451,7 +1451,57 @@ def test_document_added_properties_with_manual_capital_annotation(self): 'author')) self.assertEqual('you', list(mmif_roundtrip2.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))[ 0].get_property('author')) - + + def test_view_id_counts_rebuilt_on_deserialize(self): + # regression for issue #393: `_id_counts` is not serialized, so after + # deserialization the next generated id must not collide with an existing + # one already present in the view. + mmif = Mmif(validate=False) + doc = Document(); doc.at_type = DocumentTypes.VideoDocument + doc.id = 'd1'; doc.location = 'file:///v.mp4' + mmif.add_document(doc) + v = mmif.new_view() + v.metadata.app = tester_appname + a1 = v.new_annotation(AnnotationTypes.Annotation, document='d1', frameCount=10) + self.assertTrue(a1.id.endswith('an_1')) + # round-trip: the reloaded view still contains `an_1` but starts with an + # empty counter unless rebuilt + v2 = Mmif(mmif.serialize())[v.id] + a2 = v2.new_annotation(AnnotationTypes.Annotation, document='d1', duration=20) + self.assertTrue(a2.id.endswith('an_2')) # must not regenerate an_1 + + def test_capital_annotation_no_collision_across_apps(self): + # regression for issue #393: a downstream app re-deriving a document + # property with a *different* value must not crash on serialize; the + # distinct value is recorded as an additional capital annotation + # (value-based de-dup preserves both assertions), not a colliding one. + m = Mmif(validate=False) + d = Document() + d.at_type = DocumentTypes.VideoDocument + d.id = 'd1' + d.location = 'file:///v.mp4' + m.add_document(d) + v1 = m.new_view(); v1.metadata.app = 'http://app/producer' + v1.new_annotation(AnnotationTypes.TimeFrame, label='bars') + d.add_property('frameCount', 51248.0) + d.add_property('duration', 1709977.0) + + # downstream app loads it and re-derives a *different* duration value + m2 = Mmif(m.serialize()) + d2 = m2['d1'] + v2 = m2.new_view(); v2.metadata.app = 'http://app/consumer' + v2.new_annotation(AnnotationTypes.TimeFrame, label='caption') + d2.add_property('frameCount', 51248) # same value -> de-duped away + d2.add_property('duration', 1709975) # different value -> new annotation + out = Mmif(m2.serialize()) # must not raise KeyError + + caps = [a for view in out.views + for a in view.get_annotations(AnnotationTypes.Annotation)] + durations = {a.get_property('duration') for a in caps if 'duration' in a} + # both distinct duration assertions coexist; neither is lost or overwritten + self.assertEqual(2, len(caps)) + self.assertEqual({1709977.0, 1709975}, durations) + def test_capital_annotation_generation_viewfinder(self): mmif = Mmif(validate=False) for i in range(1, 3):