Fix: resolve major complex review - #2388
Merged
Merged
Conversation
- `DetectionDataset.__init__` lost its `isinstance(images, dict)` branch in a refactor: dict keys were stored as paths and `_images_in_memory` stayed empty, so `__getitem__`, iteration, and `merge()` on documented dict-form datasets raised "Could not read image from path" - restore the branch mirroring `ClassificationDataset.__init__`, including the deprecation warning for dict input - add 5 regression tests: getitem, len, iteration, equality, and merge round-trip on in-memory datasets --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- `MeanAveragePrecision` accepted `metric_target` but always computed IoU on boxes — mask/OBB evaluations silently returned box mAP (identical boxes with disjoint masks scored map50=1.0) - route per-detection content through `_prepare_targets`/`_prepare_predictions`: MASKS use `mask_iou_batch` with COCO crowd semantics via new `_mask_iou_with_jaccard` and mask-pixel-count area; ORIENTED_BOUNDING_BOXES use `oriented_box_iou_batch` with shoelace area; missing content raises a clear `ValueError` - BOXES path verified numerically identical to previous behavior - `ConfusionMatrix.plot()` crashed with default `normalize=False` (NaN assignment into int32 matrix); cast to float64 before masking - add regression tests: mask/OBB IoU routing, size buckets from mask area, crowd handling, empty and missing masks, winding order, raw-count and normalized plot --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- guide called five removed or renamed APIs: `PolygonZone(frame_resolution_wh=...)` (removed in 0.24.0), `BoxAnnotator` text params and `skip_label` (pre-0.22 API), `ColorPalette.default()` (now `DEFAULT`), and `VideoAssets` without import or download - snippets now run as-is against the current API --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2388 +/- ##
=======================================
Coverage 84% 84%
=======================================
Files 70 70
Lines 9946 10003 +57
=======================================
+ Hits 8339 8435 +96
+ Misses 1607 1568 -39 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores DetectionDataset support for dict-provided in-memory images (including the existing deprecation warning behavior), and also updates mAP evaluation so MeanAveragePrecision can compute IoU based on masks or oriented bounding boxes when metric_target is set accordingly.
Changes:
- Restore
DetectionDataset.__init__handling forimages: dict[str, np.ndarray]by keeping images in_images_in_memoryand emitting a deprecation warning (mirroringClassificationDataset). - Extend
MeanAveragePrecision/COCO evaluation to route IoU computation based onmetric_target(BOXES vs MASKS vs ORIENTED_BOUNDING_BOXES), and add regression tests for MASKS/OBB behavior. - Fix
ConfusionMatrix.plot()to avoid assigning NaNs into an integer matrix, and update the PolygonZone counting docs example to match current APIs.
Quality (n/5): Code Quality 4/5, Testing 4/5, Docs 3/5
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/supervision/dataset/core.py |
Restores dict-images initialization branch and deprecation warning for DetectionDataset. |
tests/dataset/test_core.py |
Adds regression coverage for dict-backed in-memory DetectionDataset behavior. |
src/supervision/metrics/mean_average_precision.py |
Adds metric-target-aware IoU computation (boxes vs masks vs OBB) in COCO evaluation path. |
tests/metrics/test_mean_average_precision.py |
Adds MASKS and OBB regression tests for MeanAveragePrecision metric_target routing and errors. |
tests/metrics/test_oriented_bounding_box_metrics.py |
Updates OBB mAP test expectations/documentation to reflect OBB IoU routing. |
src/supervision/metrics/detection.py |
Casts confusion matrix to float for plotting to prevent NaN assignment errors on int matrices. |
tests/metrics/test_detection.py |
Adds a regression test ensuring ConfusionMatrix.plot() returns a figure for int matrices. |
docs/how_to/count_in_zone.md |
Updates zone-counting tutorial code to use download_assets, ColorPalette.DEFAULT, and current PolygonZone API. |
- mean_average_precision.py:812: add .astype(np.float64) on oriented_box_iou_batch result to satisfy mypy strict mode (floating[Any] → float64) - _mask_iou_with_jaccard crowd path: replace full float64 matmul over all pairs with mask_iou_batch base (float32 + chunked) and crowd-column override only; eliminates 2× memory blowup and restores chunking safety for crowd GT inputs - dataset/core.py: add introduced (0.30.0) and removal (0.33.0) versions to DetectionDataset dict-input deprecation message per AGENTS.md §5; update class docstring to reflect deprecation --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- test_mean_average_precision.py: add TestMeanAveragePrecisionMasksCrowdBranch with crowd-GT Jaccard test and mixed crowd+normal GT test; add TestMeanAveragePrecisionMasksOrientation and extend OBB class with cross-matched 2x2 cases that fail if the (dt,gt) IoU-matrix is transposed - test_core.py: fix test_merge_preserves_in_memory_pixel_access to assert pixel correctness via public merged[0]/merged[1] not private _get_image(); add test_eq_reflexive_in_memory, test_eq_same_pixels_returns_true, test_eq_different_pixels_returns_false for in-memory DetectionDataset.__eq__ --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- docs/how_to/count_in_zone.md: drop stale VideoInfo reference from prose paragraph that no longer reflects the code after the API update - tests/metrics/test_mean_average_precision.py: rework TestMeanAveragePrecisionMasksCrowdBranch — replace all-crowd-GT scenario (returns map50=-1, no scoreable GTs) with normal+crowd GT scenarios that actually exercise the Jaccard crowd branch and produce meaningful AP --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
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.
DetectionDataset.__init__lost itsisinstance(images, dict)branch in a refactor: dict keys were stored as paths and_images_in_memorystayed empty, so__getitem__, iteration, andmerge()on documented dict-form datasets raised "Could not read image from path"ClassificationDataset.__init__, including the deprecation warning for dict input