fix(dataset): stop split mutation, determinize VOC, guard collisions - #2394
Merged
Conversation
- `train_test_split` seeded the global `random` module and shuffled the caller's list in place, so `DetectionDataset.split()` reordered its own `image_paths` and polluted process-wide randomness; use a local `random.Random` and shuffle a copy - Pascal VOC class ids were assigned in `set`-iteration and filesystem-glob order, so the same dataset produced different `class_id` values across runs; sort class names and the loaded file list - dataset exports keyed output files on basename, silently overwriting when two entries shared a name across directories (common after `merge()`); detect basename collisions and raise - add regression tests for split determinism, VOC id stability, and export collisions --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2394 +/- ##
=======================================
Coverage 85% 85%
=======================================
Files 70 70
Lines 10013 10035 +22
=======================================
+ Hits 8467 8489 +22
Misses 1546 1546 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves dataset utility determinism and safety across DetectionDataset splitting/loading/exporting by eliminating global RNG side effects, making Pascal VOC class-id assignment stable, and preventing silent overwrites during flat-directory exports.
Changes:
- Make
train_test_split()RNG-isolated (no globalrandomseeding) and avoid mutating the caller’s input list. - Determinize Pascal VOC loading by sorting discovered image files and sorting per-file discovered class names before extending the class list.
- Add export-time guards to detect basename/stem collisions before writing images/labels/XML, plus regression tests covering all three behaviors.
Quality assessment (1–5):
- Code quality: 4/5
- Tests: 5/5
- Docs: 4/5
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/supervision/dataset/utils.py |
Adds collision guard helper and makes train_test_split() deterministic/isolation-friendly. |
src/supervision/dataset/formats/pascal_voc.py |
Ensures stable image traversal order and stable class-id assignment. |
src/supervision/dataset/formats/yolo.py |
Prevents YOLO label overwrites by validating output-name uniqueness up front. |
src/supervision/dataset/core.py |
Adds collision checks for Pascal VOC export annotations. |
tests/dataset/test_utils.py |
Adds regression tests for split RNG isolation and collision detection helper. |
tests/dataset/test_core.py |
Adds regression test ensuring as_yolo() raises on colliding basenames. |
tests/dataset/formats/test_pascal_voc.py |
Adds regression tests validating deterministic VOC class ordering and stable class-ids across runs. |
…ake guard private - Make the basename collision guard private and update internal call sites. - Add pre-flight collision checks before dataset export writes. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: OpenAI Codex <codex@openai.com>
…and boundary cases - Cover YOLO and Pascal VOC collision guard paths. - Add empty and single-path boundary cases for the private guard. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: OpenAI Codex <codex@openai.com>
…non-mutation guarantee - Document export collision ValueError cases and split non-mutation. - Fix Pascal VOC loader/parser docs and add a train/test split example. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: OpenAI Codex <codex@openai.com>
Borda
added a commit
that referenced
this pull request
Jul 3, 2026
Resolve duplicate-change conflicts: several commits from this branch were cherry-picked into separate PRs (#2391/#2393/#2394/#2396/#2397/#2398/#2399) and squash-merged to develop, so merging develop back produced same-change conflicts across 19 files. Resolution: - Keep this branch's later refactors: public `check_no_basename_collisions` (standardized collision-guard layer, renamed all `_check_*` call sites in dataset/core, yolo, labelme, tests), `save_pascal_voc_annotations` helper, value-based from_lmm->from_vlm mapping (drop dead lmm_to_vlm dict), CropAnnotator source-scene snapshot for overlapping boxes. - Take develop's hardening/refinements: from_vlm empty-list guard, explicit float() mAP casts, np.float32 dtypes, MJPG/avi VideoSink tests, shared make_panoptic_png test helper, expanded empty-input adapter tests. - Combine where both improve: np.int32 crop dtype + snapshot; keep both sides' added tests. Full suite: 2924 passed, 1 xfailed. pre-commit (ruff, mypy, codespell) clean. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Draft
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.
train_test_splitseeded the globalrandommodule and shuffled the caller's list in place, soDetectionDataset.split()reordered its ownimage_pathsand polluted process-wide randomness; use a localrandom.Randomand shuffle a copyset-iteration and filesystem-glob order, so the same dataset produced differentclass_idvalues across runs; sort class names and the loaded file listmerge()); detect basename collisions and raise