Skip to content

Conversation

@keighrim
Copy link
Member

fixes #296

@clams-bot clams-bot added this to infra Jun 14, 2025
@github-project-automation github-project-automation bot moved this to Todo in infra Jun 14, 2025
@codecov
Copy link

codecov bot commented Jun 14, 2025

Codecov Report

Attention: Patch coverage is 76.19048% with 35 lines in your changes missing coverage. Please review.

Project coverage is 87.74%. Comparing base (8a6a942) to head (711bc35).
Report is 63 commits behind head on develop.

Files with missing lines Patch % Lines
mmif/serialize/view.py 67.39% 15 Missing ⚠️
mmif/serialize/mmif.py 79.31% 12 Missing ⚠️
mmif/serialize/annotation.py 84.21% 6 Missing ⚠️
mmif/utils/video_document_helper.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #318      +/-   ##
===========================================
- Coverage    92.59%   87.74%   -4.86%     
===========================================
  Files           10       13       +3     
  Lines         1405     1648     +243     
===========================================
+ Hits          1301     1446     +145     
- Misses         104      202      +98     
Flag Coverage Δ
unittests 87.74% <76.19%> (-4.86%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@keighrim keighrim force-pushed the 296-always-longid branch from 59695d8 to 3b23a17 Compare June 15, 2025 01:54
@keighrim keighrim force-pushed the 296-always-longid branch 3 times, most recently from 79241fb to 711bc35 Compare July 1, 2025 16:56
- `Annotation.id` property now must be in the "long" form, providing better disambiguation.
    - automatically generated IDs will be in the long form.
    - old MMIF files (specver 1.0.x) are sill supported, as long as the ID ambiguity is reasonably "solvable" see `View._fix_old_short_ids()` methods.
- DEPRECATED `Annotation.long_id` getter/setter, and `Annotation._short_id` (private) getter/setter.
- `Mmif.__getitem__()` (`[]` indexing) no longer raises "ambiguous id" error.
- (possibly breakinig change) For dict-like sub-fields, behavior of `__getitem__()` and `get()` are updated for consistency with Python.
    - When the key is not found, `get()` will return a default value (which is `None` by default), while `__getitem__` raises `KeyError`
    - `Annotation` (search for explicit (in its own propoerties) prop key and implicit (in contains metadata) prop key)
    - `AnnotationProperties` (straightforwardly search for prop key)
    - `View` (straightforwardly search for top-level field names, or ID in `annotations`)
    - `Mmif` (search for top-level field names, or for ID in `documents`, `views`, all `views[].annotations`)
- DEPRECATED `get_X-by_id()` methods, `[]` indexing method should be enough
- Simplified `Annotation.parent` getter, DEPRECATED its setter (no longer keep them in memory as ID string can serve the purpose)
- Additional validations in place
    - When an annotation is added to a view, its ID is validated against view ID (check if correctly prefixed)
    - When a view is added to a mmif, its ID is checked for existing duplicates
- Updated test cases to handle the change in ID (or marked skip if not properly manageable)
@keighrim keighrim force-pushed the 296-always-longid branch from 711bc35 to d814030 Compare July 22, 2025 23:03
@keighrim keighrim merged commit e0b811f into develop Jul 22, 2025
1 check failed
@keighrim keighrim deleted the 296-always-longid branch July 22, 2025 23:21
@github-project-automation github-project-automation bot moved this from Todo to Done in infra Jul 22, 2025
@keighrim keighrim mentioned this pull request Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Update mmif.serialize.annotation.Annotation:id to always use "long" form

2 participants