Skip to content

fix(detection): support float64 and arbitrary dtypes in polygon utilities - #2542

Merged
Borda merged 3 commits into
roboflow:developfrom
pranaysb:fix/polygon-dtype-support
Sep 7, 2026
Merged

fix(detection): support float64 and arbitrary dtypes in polygon utilities#2542
Borda merged 3 commits into
roboflow:developfrom
pranaysb:fix/polygon-dtype-support

Conversation

@pranaysb

@pranaysb pranaysb commented Sep 5, 2026

Copy link
Copy Markdown
Contributor
Before submitting
  • Self-reviewed the code
  • Updated documentation, follow Google-style
  • Added docs entry for autogeneration (if new functions/classes)
  • Added/updated tests
  • All tests pass locally

Description

Support arbitrary polygon dtypes (such as np.float64, NumPy's default float, as well as int64, int16, uint8) in filter_polygons_by_area and approximate_polygon.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)

Motivation and Context

cv2.contourArea and cv2.approxPolyDP require inputs of depth CV_32F (np.float32) or CV_32S (np.int32). When users pass standard NumPy polygons (which default to np.float64 or np.int64 on 64-bit systems), OpenCV throws a C++ assertion failure:

cv2.error: OpenCV(4.x.x) ...: error: (-215:Assertion failed) npoints >= 0 && (depth == CV_32F || depth == CV_32S) in function 'contourArea'

In addition, filter_polygons_by_area had a minor typo referencing ares instead of areas.

Changes Made

  • In filter_polygons_by_area, pass np.float32 copy to cv2.contourArea if the input polygon is not float32 or int32. Fixed typo ares -> areas.
  • In approximate_polygon, ensure OpenCV receives float32/int32, while preserving and returning points with the input array's original dtype.
  • Added parameterized unit tests in tests/detection/utils/test_polygons.py validating behavior across np.float64, np.float32, np.int64, np.int32, np.int16, and np.uint8.

Testing

  • I have tested this code locally
  • I have added unit tests that prove my fix is effective or that my feature works
  • All new and existing tests pass (43/43 tests in tests/detection/utils/test_polygons.py)
  • ruff check and ruff format --check passed

@pranaysb
pranaysb requested a review from SkalskiP as a code owner September 5, 2026 12:19
@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88%. Comparing base (badd869) to head (d72ac3e).

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #2542   +/-   ##
=======================================
  Coverage       88%     88%           
=======================================
  Files           85      85           
  Lines        12198   12208   +10     
=======================================
+ Hits         10734   10744   +10     
  Misses        1464    1464           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Borda
Borda requested a balanced review from Copilot September 7, 2026 11:39
@Borda Borda added the bug Something isn't working label Sep 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Absolute float32 conversion corrupts large-coordinate polygons, and the required changelog entry is missing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds broader NumPy dtype support to polygon area filtering and approximation.

Changes:

  • Converts unsupported OpenCV dtypes while preserving returned dtypes.
  • Adds parametrized dtype tests.
  • Fixes the ares typo.

Review: Code quality 2/5; testing 3/5; documentation 2/5; risk 3/5. Large-coordinate precision and changelog issues remain.

File summaries
File Description
src/supervision/detection/utils/polygons.py Adds dtype conversion for OpenCV polygon operations.
tests/detection/utils/test_polygons.py Tests supported input and output dtypes.
Review details

Suppressed comments (1)

tests/detection/utils/test_polygons.py:188

  • Repository test guidance requires non-primitive parametrized cases such as dtype classes to use pytest.param(..., id="semantic-slug") rather than bare values (.github/CONTRIBUTING.md:342). Use explicit IDs here as well so parametrized case names remain stable and clear.
    [np.float64, np.float32, np.int64, np.int32, np.int16, np.uint8],
  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/supervision/detection/utils/polygons.py Outdated
Comment thread src/supervision/detection/utils/polygons.py Outdated
Comment thread src/supervision/detection/utils/polygons.py Outdated
Comment thread tests/detection/utils/test_polygons.py Outdated
Changes:
- Rebase polygon coordinates before OpenCV conversion and restore approximated integer coordinates with overflow-safe arithmetic.
- Add int64/float64 large-origin regressions, stable dtype parameter IDs, and an Unreleased changelog entry.

Impact:
- Area filtering and polygon approximation retain small local geometry at large absolute coordinates instead of collapsing float32 deltas.
- Tests have readable stable case IDs and the user-visible correction is documented.

Verification:
- `uv run pytest -q`: 3840 passed, 1 skipped.
- Ruff lint and format checks passed; `git diff --check` passed.

Residual limits:
- Local mypy is unavailable; independent review provenance and Codemap context remain outstanding.

---

Co-authored-by: Codex <codex@openai.com>
@Borda
Borda merged commit fbd7655 into roboflow:develop Sep 7, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants