Repository navigation
fix(key_points): preserve empty-joint skeletons during NMS - #2639
Conversation
@INo-xious ^^ 🙏 |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused fix preserves existing validation and NMS behavior with thorough regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes KeyPoints.with_nms() for nonempty skeleton batches with zero joints.
Changes:
- Uses neutral reduction sentinels for empty joint axes.
- Adds comprehensive regression and validation tests.
- Updates documentation and changelog.
Assessment: Code quality 5/5 · Testing 5/5 · Documentation 5/5 · Risk 1/5
| File | Description |
|---|---|
src/supervision/key_points/core.py |
Handles empty joint-axis reductions safely. |
tests/key_points/test_core.py |
Covers preservation, aligned fields, and validation. |
docs/changelog.md |
Documents the bug fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2639 +/- ##
=======================================
Coverage 92% 92%
=======================================
Files 78 78
Lines 11733 11733
=======================================
Hits 10759 10759
Misses 974 974 🚀 New features to boost your workflow:
|
Changes: - Integrate develop updates for float64 COCO-compatible mAP thresholds, in-place dataset exports, and video writer error propagation, with their regression tests. - Resolve the Unreleased changelog conflict by retaining the PR roboflow#2639 entry and all three target entries. Impact: - The PR branch now contains the fetched target fixes and keeps the keypoint NMS release note alongside target release notes. - The merge preserves target implementations and tests without mixing in review finding changes. Verification: - Focused keypoint, dataset, metrics, and video tests: 331 passed. - git diff --check passed; unresolved merge paths: none. Residual limits: - The fresh review's local mypy gate remains failed because the cached environment lacks project dependencies and third-party stubs. - codecov/project was pending at review collection. --- Co-authored-by: Codex <[email protected]>
* fix(key_points): preserve empty-joint skeletons during NMS * docs: link empty-joint NMS fix pull request --------- Co-authored-by: jirka <[email protected]> Co-authored-by: Codex <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
KeyPoints.with_nms()raises a zero-size reduction error after boolean filtering removes every joint from a nonempty batch. It now preserves those skeletons and their aligned fields, consistent with skeletons whose joints are all unusable.Motivation and Context
Public keypoint filtering can produce valid
(N, 0, 2)coordinates. The bounding-box reductions ran before the existing no-valid-joints handling and could not reduce the empty joint axis.#2401 and merged #2402 addressed indexing and
as_detections. Merged #2563 addressed nonfinite joints in NMS. Open #2633 populates detection confidence in pose connectors. This change addresses the separate empty-axis failure.Changes Made
Testing
All commands ran in this branch's independent worktree based on
8c7df2388ce46f16139edc498c2bc3874493b4bc.Clean base:
uv run pytest --cov=supervision— 4316 passed, 24 skipped; 91% coverage.New regression class against unchanged implementation:
uv run pytest tests/key_points/test_core.py::TestKeyPointsWithNmsEmptyJoints— 8 failed with the expected zero-size reduction error (including two masked threshold-validation errors), 3 existing required-field checks passed.Fixed NMS cases:
uv run pytest tests/key_points/test_core.py -k 'nms or Nms'— 33 passed.Full suite:
uv run pytest --cov=supervision— 4327 passed, 24 skipped; 91% coverage.Relevant doctests:
uv run pytest --doctest-modules src/supervision/key_points/core.py— 10 passed.uv run pre-commit run --all-files— all 22 hooks passed after formatting.git diff --check— passed.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
Validation used Python 3.13.9 and NumPy 2.3.1 with the supported OpenCV-free backend. The same 24 pre-existing skips remained before and after: 23 require optional OpenCV and one requires optional rasterio. No public API or dependency changes.