Correct label-count return types/docs and remove broken VideoLabels.counts - #430
Merged
Conversation
Project.load_counts returns a dict keyed by integer identity whose values are
dicts of named count tuples, but its annotation claimed
dict[str, tuple[int, int]] and Project.counts documented an obsolete list-of-
5-tuples shape. Both are corrected to match what every caller actually
consumes.
The per-identity loop unions the identity keys of the "labels" and
"unfragmented_labels" sections but defaulted a missing identity to [], which
count_labels then calls .get() on. Default to {} instead so an identity present
in only one section counts as zero frames and zero bouts rather than raising
AttributeError. The previous "if labels" guard becomes redundant and is
removed; it produced the same ((0, 0), (0, 0)) result.
VideoLabels.counts() described the obsolete shape and read a TrackLabels.counts
attribute that does not exist, so it raised AttributeError for any labeled
identity. It has no callers and is removed.
Also fixes the identity key type on project_pruning.check_label_counts and adds
regression tests for load_counts.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes mismatches between the documented/annotated label-count API and its actual runtime shape, and removes a dead/broken VideoLabels.counts() method that could only raise at runtime. It primarily improves correctness of type hints/docs around Project.load_counts() / Project.counts() and adds regression coverage for an AttributeError edge case in load_counts.
Changes:
- Corrected
Project.load_counts()andProject.counts()type annotations/docstrings to match the actual returned structure (per-video → per-identity → named count tuples). - Fixed a latent
load_counts()crash by defaulting missing identity sections to{}(instead of[]) when counting. - Removed unused/broken
VideoLabels.counts()and added regression tests forload_counts()identity handling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/jabs/project/project.py |
Fixes counts/load_counts signatures/docs and hardens load_counts defaults to avoid AttributeError. |
src/jabs/project/video_labels.py |
Removes an unused counts() method that referenced a non-existent TrackLabels.counts. |
src/jabs/project/project_pruning.py |
Aligns helper type hints with the corrected load_counts() return shape (int identity keys). |
tests/project/test_project.py |
Adds regression tests ensuring identity keys are int and missing-section identities count as zero. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
The label-count API (
Project.load_counts→Project.counts) is consumed in sixplaces (
central_widget,training_strategy,session_tracker,project_pruning,cross_validationCLI,MultiClassClassifier), and every oneof them treats the result as
dict[identity_int, dict[str, tuple[int, int]]].The declarations disagreed with that in three ways, all of which actively mislead
a reader:
Project.load_countswas annotated-> dict[str, tuple[int, int]]. Both the key type and the value type arewrong — keys are
int(the code doescounts[int(identity)] = {...}) andvalues are dicts of four named count tuples. The docstring body below it
already described the correct shape, so the annotation contradicted the
docstring immediately above the code.
Project.countsdocumented its values as"lists of (identity, 4 tuples)" — a shape that no longer exists anywhere.
VideoLabels.countsis where thatobsolete shape came from, and it is broken: it reads
TrackLabels.counts, an attributeTrackLabelsdoes not define. Any callwith a labeled identity raises
AttributeError: 'TrackLabels' object has no attribute 'counts'(verified againstmain). It has no callers insrc/,tests/, orpackages/.While confirming the real shape I also found a latent crash in
load_counts.The loop iterates the union of identity keys from the
labelsandunfragmented_labelssections, but defaults a missing identity to[]— andcount_labelsimmediately calls.get()on its argument. An identity present inone section but not the other therefore raises
AttributeError: 'list' object has no attribute 'get'. Files written byVideoLabels.as_dictcurrently keep both sections in sync (it seeds{}forevery identity in both), so this is unreachable today, but it is one asymmetric
annotation file away from a hard failure at project load. A wrong-typed default
constant is exactly the kind of thing that stops being latent when the writer
changes.
I picked this over the other candidates I looked at (a copy-paste "ignored"
docstring on
psd_mean_band'sfreqsparameter, which is used; theTrackLabels.downsampledocstring's imprecise PAD bin description) because thisone is a genuine bug plus documentation that would send a reader down the wrong
path about a widely-consumed return type — not just cosmetics.
Why it's safe: net-zero behavior change on every reachable path.
count_labels({})returns((0, 0), (0, 0)), which is exactly what the removedif labels else ((0, 0), (0, 0))guard produced, so the guard is redundant, notdropped semantics. Annotations and docstrings do not affect runtime. The removed
VideoLabels.countscould only ever raise. NoFEATURE_VERSIONbump — nocomputed feature values are touched.
Change
Logic changes
src/jabs/project/project.pyload_counts: corrected return annotation todict[int, dict[str, tuple[int, int]]]; added the missingArgssection;changed the two missing-identity defaults from
[]to{}and removed thenow-redundant
if labelsguard (the AttributeError fix).counts: addedbehavior: strand the return annotation; replaced theobsolete list-of-tuples
Returnsdescription with a pointer toload_counts.src/jabs/project/video_labels.py— removed the unused, always-raisingcounts()method.src/jabs/project/project_pruning.py— corrected the identity key type onthe local
check_label_countshelper (dict[str, ...]→dict[int, ...]) tomatch what
load_countsreturns.tests/project/test_project.py— two regression tests forload_counts:one asserting identities come back as
intwith both fragmented andunfragmented counts, one covering an identity present only in
unfragmented_labels. The second fails onmainwith theAttributeErrorabove.
Mechanical updates
None — no import paths, exports, or re-export shims were touched.
Verification
ruff check .andruff format .clean; full root suite827 passed, 200 skipped. Nothing underpackages/was modified.This PR was produced by an automated analysis from Claude Code.
Generated by Claude Code