Allow a dataset record to carry multiple detections - #469
Conversation
Move the LDF annotation models and the ParquetRecord row schema into a new lightweight `luxonis_ml.ldf` package with its own extra, so consumers can depend on the data format without pulling in the full data stack. `luxonis_ml.data.datasets.annotation` and `luxonis_ml.data.utils.parquet` re-export the moved names, so existing imports keep working. Pure move: no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`DatasetRecord.annotation` becomes `list[Detection] | None`; a single `Detection` is still accepted and upgraded to a one-element list, so existing generators keep working. Detections are flattened into the same parquet rows, and each secondary media source still gets exactly one null-annotation row rather than one per detection. `add()` infers the record's task from every detection and refuses to guess when their classes belong to different tasks, instead of silently filing them all under the first detection's task. The parser's task splitter groups a record's detections by resolved task name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds multi-detection support across dataset records, parsers, ingestion, array processing, and parquet serialization. Task grouping and inference now process every annotation, with validation for ambiguous task mappings. ChangesMulti-annotation support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DatasetRecord
participant BaseParser
participant LuxonisDataset
participant ParquetConversion
DatasetRecord->>BaseParser: Provide scalar or list annotations
BaseParser->>BaseParser: Group detections by resolved task
BaseParser->>LuxonisDataset: Yield task-specific record copies
LuxonisDataset->>LuxonisDataset: Infer task and process every annotation
LuxonisDataset->>ParquetConversion: Serialize all detections and nested data
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Formatting only, as taplo writes it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Formatting only, as taplo writes it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #469 +/- ##
==========================================
+ Coverage 95.03% 95.08% +0.05%
==========================================
Files 177 178 +1
Lines 14557 14673 +116
==========================================
+ Hits 13834 13952 +118
+ Misses 723 721 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
`DatasetRecord.annotation` becomes `list[Detection] | None`; a single `Detection` is still accepted and upgraded to a one-element list, so existing generators keep working. Detections are flattened into the same parquet rows, and each secondary media source still gets exactly one null-annotation row rather than one per detection. `add()` infers the record's task from every detection and refuses to guess when their classes belong to different tasks, instead of silently filing them all under the first detection's task. The parser's task splitter groups a record's detections by resolved task name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1c54c6f to
295bf66
Compare
klemen1999
left a comment
klemen1999
left a comment
There was a problem hiding this comment.
Possible issue found in the NativeParser:
If one follows the new DatasetRecord format that supports multiple annotations then current NativeParser can crash. The problem is here where we still assume records with single annotation. This is still true for datasets exported by us but if one constructs LDF by themselves they could now have multiple records. We should handle this in the parser
|
And maybe an important breaking change node to this PR is that something like this: doesn't work anymore since now |
…/luxonis-ml into feat/record-multiple-annotations
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@luxonis_ml/ldf/annotation.py`:
- Around line 1464-1471: Update LuxonisDataset._process_arrays to recursively
traverse each detection’s sub_detections before serialization, processing every
nested detection.array with the same local-path and remote-upload/rewrite
behavior as root detections. Ensure nested arrays are no longer left relative or
unuploaded, and add regression coverage for both local and remote datasets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a6bcf9b-53b1-4116-b8b3-0a40e33d43c6
📒 Files selected for processing (8)
luxonis_ml/data/datasets/luxonis_dataset.pyluxonis_ml/data/parsers/base_parser.pyluxonis_ml/data/parsers/native_parser.pyluxonis_ml/ldf/annotation.pytests/test_data/test_native_paths.pytests/test_data/test_parsers.pytests/test_data/test_task_ingestion.pytests/test_ldf/test_record_list.py
🚧 Files skipped from review as they are similar to previous changes (7)
- luxonis_ml/data/parsers/native_parser.py
- tests/test_ldf/test_record_list.py
- tests/test_data/test_native_paths.py
- tests/test_data/test_task_ingestion.py
- luxonis_ml/data/parsers/base_parser.py
- luxonis_ml/data/datasets/luxonis_dataset.py
- tests/test_data/test_parsers.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_data/test_dataset.py`:
- Around line 566-569: Strengthen the remote-path assertions in the dataset test
by comparing root and nested against the expected UUID-derived filenames from
storage metadata or the upload API, rather than only checking each path’s
basename. Use the _process_arrays contract and available expected UUID values to
verify both uploaded array paths were renamed correctly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3994bd23-c409-4901-b673-f7a9e1036162
📒 Files selected for processing (2)
luxonis_ml/data/datasets/luxonis_dataset.pytests/test_data/test_dataset.py
🚧 Files skipped from review as they are similar to previous changes (1)
- luxonis_ml/data/datasets/luxonis_dataset.py
2 participants
Purpose
A
DatasetRecordcould hold only oneDetection, so a sample with several objects had to be yielded as several records that repeated the same file and metadata. Grouping them in one record is the natural shape, and it is what reconstructing records from loader output needs.Split out of the
feat/vizlabbranch, which had grown to 81 commits; this is one of eight self-contained pieces that do not belong to vizlab itself.Specification
DatasetRecord.annotationbecomeslist[Detection] | None. A singleDetectionis still accepted and upgraded to a one-element list by a before-validator, so existing generators keep working unchanged.LuxonisDataset.add()infers the task from every detection and raises when their classes belong to different tasks, instead of inferring from the first one and silently filing the rest under it — which would pollute that task's class list and mis-group labels that training then consumes as ground truth.Dependencies & Potential Impact
Backwards compatible for producers (single detections still work) and for the on-disk format (identical rows). One deliberate new failure mode: a record whose detections resolve to different tasks now raises instead of guessing.
Deployment Plan
None / not applicable
Testing & Validation
tests/test_ldf/test_record_list.py(single vs. list equivalence, flattening, one null row per secondary source, sub-detections), new task-inference coverage intests/test_data/test_task_ingestion.py, and grouping coverage intests/test_data/test_parsers.py.AI Usage
Assisted-by: Claude:claude-fable-5
Summary by CodeRabbit
New Features
Bug Fixes
Tests