Correct the documented on-disk annotation file format - #461
Open
gbeane wants to merge 2 commits into
Open
Conversation
The docstring on VideoLabels.as_dict is the reference for the on-disk jabs/annotations/<video>.json format, and it was wrong in several ways: - the optional identity field of a timeline annotation was documented as "animal_id", a key that appears nowhere in the code - the "external_identities" example was malformed and had the mapping backwards - the "version" key that as_dict always writes was undocumented - "labels" and "unfragmented_labels" were shown with identical contents and no explanation of the difference between them Rewrite it to document each key, note that load() prefers "unfragmented_labels", and give a valid example. Document the fields of a serialized annotation in TimelineAnnotations.serialize, where they are produced, and correct the external_identities type in the PoseEstimation class docstring (list[str], not list[int]). Documentation only, no behavior change.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Documentation-only updates align with current serialization behavior, and the remaining feedback is a minor clarity nit.
Pull request overview
This PR corrects and clarifies the documented on-disk JSON format for JABS per-video annotation files (jabs/annotations/<video>.json) by updating the relevant docstrings to match what the code actually serializes/deserializes.
Changes:
- Rewrites
VideoLabels.as_dict()docstring to accurately document top-level keys (includingversion), fix the identity key name (identity), and explainlabelsvsunfragmented_labels. - Documents the serialized fields produced by
TimelineAnnotations.serialize()and consumed byTimelineAnnotations.load(). - Fixes the
PoseEstimationdocstring type/meaning ofexternal_identitiesto reflectlist[str] | None.
File summaries
| File | Description |
|---|---|
| src/jabs/project/video_labels.py | Updates as_dict() docstring to match the real JSON annotation file structure and key semantics. |
| src/jabs/project/timeline_annotations.py | Documents the serialized annotation entry fields and clarifies what is (and isn’t) persisted. |
| packages/jabs-core/src/jabs/core/abstract/pose_est.py | Corrects external_identities docstring to `list[str] |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses review feedback: the example in VideoLabels.as_dict is the serialized JSON file, so its `true` is JSON rather than a Python literal. Say so explicitly instead of leaving the reader to infer it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reasoning
I went looking for a bug first and did not find one I was confident in, so I picked the next
category: misleading documentation. The docstring on
VideoLabels.as_dictis the only place in the tree that documents the layout of a project's
jabs/annotations/<video>.jsonfile, so anyone reading a JABS annotation file from an externaltool ends up here. It was wrong in four ways:
field as
"animal_id": 0. The actual key written byTimelineAnnotations.serializeis
"identity".animal_idappears nowhere else insrc/orpackages/-- a consumerfollowing the docstring would silently never find the identity.
external_identitiesexample was malformed and backwards. It read"external_identities: { "jabs identity", 1234, }-- an unterminated key and a comma insteadof a colon -- and implied the mapping went from a name to a number. The code writes
{str(jabs_identity_index): external_identity}.versionwas undocumented.as_dicthas written a"version"key(
SERIALIZED_VERSION) since it was added, but the example never showed it.labelsvsunfragmented_labelswas unexplained. Both were shown with byte-identicalcontents, which reads as pointless duplication. In fact
labelsis masked bypose.identity_mask()(blocks split where the identity is absent from the pose file) whileunfragmented_labelsis what the user actually labeled, andload()prefersunfragmented_labels. That distinction is the single most important thing about this formatand it was missing.
This is safe because it is comments only: no statement, expression, signature, or default was
touched, so behavior is net-zero by construction. Nothing in the tree parses these docstrings.
Change
Logic changes
None -- every edit is inside a docstring.
Documentation changes
src/jabs/project/video_labels.py-- rewrote theas_dictdocstring. It now has properArgs/Returnssections, documents each top-level key (includingversion), spells out thelabels/unfragmented_labelsdifference and which oneload()prefers, notes thatexternal_identitiesandannotationsare only present conditionally, points atTimelineAnnotations.serializefor annotation entry fields, and carries a valid JSON examplewith
"identity"instead of"animal_id".src/jabs/project/timeline_annotations.py-- documented the serialized fields of oneannotation (
start,end,tag,color, optionaldescription, optionalidentity) onserialize(), where they are produced, and noted that the deriveddisplay_identityis notserialized because
load()recomputes it.packages/jabs-core/src/jabs/core/abstract/pose_est.py-- corrected theexternal_identitiesentry in thePoseEstimationclass docstring: the property is annotatedand populated as
list[str] | None, notlist[int] | None(the v4+ readers stringify the rawids).
No mechanical/import churn in this PR.
Verification
uv run --only-group lint ruff check-- all checks passeduv run --only-group lint ruff format --check-- 460 files already formatteduv run pytest-- 879 passed, 254 skippeduv run pytest packages/jabs-core/tests-- 88 passed🤖 Generated with Claude Code
Generated by Claude Code