Repository navigation
fix(detection): index list data with a list of booleans as a mask - #2737
Open
MohammadHijjawi97 wants to merge 2 commits into
Open
MohammadHijjawi97 wants to merge 2 commits into
MohammadHijjawi97 wants to merge 2 commits into
Conversation
`Detections[[True, False, True]]` selects rows 0 and 2 of every array field, because NumPy reads a list of booleans as a mask, but `get_data_item` read the same list as positions for list-valued `data`: `True` and `False` became the indices 1 and 0. An all-True mask therefore returned the second item for every row, any other mask failed with a length error, and a list of `np.bool_` values, as from `[c == 0 for c in detections.class_id]`, raised `TypeError`. List data now takes the positions NumPy resolves for the same index.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Single-row selections now allocate full-length position arrays, introducing quadratic work when smoothing detections with list-valued data.
1 open finding
What changed in this PR
Fixes boolean-list indexing so list-valued detection metadata stays aligned with NumPy array fields.
Changes:
- Resolves list indices through NumPy.
- Adds regression tests for mixed, all-true, all-false, and NumPy boolean masks.
- Documents the fix in the changelog.
| File | Description |
|---|---|
| tests/detection/test_core.py | Adds boolean-list selection tests. |
| src/supervision/detection/utils/internal.py | Corrects mask handling for list-valued data. |
| docs/changelog.md | Documents corrected selection behavior. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+765
to
+768
| # Resolve the positions as NumPy indexes the array fields, so a | ||
| # list of booleans is a mask, not the positions 0 and 1. | ||
| positions = np.arange(len(value))[index] | ||
| subset_data[key] = [value[i] for i in positions] |
This branch has not been deployed
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.

Description
Detections.__getitem__accepts a list of booleans as a mask: NumPy selects the matching rows ofxyxy,mask,confidence,class_id,tracker_idand every array indata. List values indataare a supported type, butget_data_itemread the same list as positions, soTrueandFalsebecame the indices1and0. Depending on the mask this gave wrong data, aValueError, or aTypeError.On
develop:With this change:
Motivation and Context
Filtering with a list comprehension, such as
detections[[c in wanted for c in detections.class_id]], is a common way to select detections. With list-valueddatait silently scrambled labels when every element wasTrue, and failed otherwise. The array fields of the sameDetectionswere already selected correctly, so only the list data was out of step.Changes Made
get_data_item: for a Python list index, list data now takes the positions NumPy resolves for that index (np.arange(len(value))[index]). A list of booleans, includingnp.bool_elements, now acts as a mask, as it does for the array fields. Integer lists, negative indices and out-of-range errors behave as before.np.ndarrayindices are unchanged.TestDetectionsSelectBooleanListintests/detection/test_core.py.Testing
pytest tests/detection/test_core.py -k BooleanList: all 4 cases fail ondevelopand pass with this change.pytest tests: 5603 passed, 1 skipped.pytest --doctest-modules src/supervision/detection/core.py src/supervision/detection/utils/internal.py: 16 passed.pre-commit run --fileson the changed files: ruff, ruff-format, docformatter and codespell pass. mypy reports no errors in the changed files.Added/updated tests, and the full suite passes locally
Updated docs (docstrings / mkdocs entry) for new or changed public API
Added a changelog entry in
docs/changelog.mdunderUnreleased(skip for lint/type/format-only or pure doc changes)Additional Notes
KeyPointsuses the same helper, so list data inKeyPoints.datagets the same fix.