feat(detection): add require_all_anchors to PolygonZone#2272
Conversation
Ace3Z
force-pushed
the
feat/polygon-zone-require-all-anchors
branch
from
1e0e297 to
80b0181
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2272 +/- ##
=======================================
Coverage 78% 78%
=======================================
Files 66 66
Lines 8410 8413 +3
=======================================
+ Hits 6552 6555 +3
Misses 1858 1858 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a require_all_anchors switch to sv.PolygonZone so callers can choose whether all configured anchors must be inside the polygon (current/default behavior) or whether any anchor being inside is sufficient—addressing overlap/straddling use-cases from #1022.
Changes:
- Add
require_all_anchors: bool = TruetoPolygonZoneand switch reduction fromnp.alltonp.anywhen disabled. - Add a unit test validating
require_all_anchors=Falsetriggers when only one of multiple anchors is inside.
Holistic assessment (n/5):
- Code quality: 3/5 (small change, but current
Iterablehandling can introduce a real runtime bug) - Testing: 4/5 (good targeted test added)
- Docs: 4/5 (docstring updated for new flag)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/supervision/detection/tools/polygon_zone.py | Adds require_all_anchors and updates trigger logic to support “any anchor” semantics. |
| tests/detection/test_polygonzone.py | Adds a regression test for the new require_all_anchors=False behavior. |
| self.polygon = polygon.astype(int) | ||
| self.triggering_anchors = triggering_anchors | ||
| if not list(self.triggering_anchors): | ||
| raise ValueError("Triggering anchors cannot be empty.") | ||
| self.require_all_anchors = require_all_anchors |
There was a problem hiding this comment.
Good catch. Fixed by materializing the iterable once with list(triggering_anchors) at the top of init, and added a regression test that passes a generator and asserts trigger() still works. Pushed in the latest commit.
| """With require_all_anchors=False, any anchor inside triggers.""" | ||
| # Box [85, 85, 115, 115] has only BOTTOM_RIGHT (115, 115) inside POLYGON | ||
| # ([100, 100]..[200, 200]); the other three corners are outside. | ||
| detections = _create_detections(xyxy=[[85.0, 85.0, 115.0, 115.0]], class_id=[0]) |
There was a problem hiding this comment.
That line measures 86 characters including leading indent, which is under the configured 88-character limit. ruff format also leaves it alone in pre-commit, both locally and in pre-commit.ci on this PR. Looks like a Copilot false positive.
Currently a detection counts as 'in the zone' only when every anchor in triggering_anchors is inside. For boxes that straddle the zone boundary this means a detection with many anchors (e.g. the four corners) is often under-counted unless the user shrinks triggering_anchors to a single point. Add require_all_anchors: bool = True so callers can opt into 'any anchor inside is enough'. Default preserves current behaviour. Closes roboflow#1022.
Ace3Z
force-pushed
the
feat/polygon-zone-require-all-anchors
branch
from
80b0181 to
f5ee545
Compare
|
Friendly ping. @Borda, would you have a moment to take a look? Happy to address any feedback. |
Closes #1022.
Currently a
PolygonZonecounts a detection as inside only when every anchor intriggering_anchorsis inside the polygon. For boxes that straddle the zone boundary this is often the wrong default: a detection with multiple anchors (e.g. all four corners) gets dropped unless the caller manually shrinkstriggering_anchorsto a single point. The reporter and @SkalskiP discussed this in #1022 and @SkalskiP invited a PR for exactly this knob.Add a constructor flag:
When
True(the default) the behaviour is unchanged. WhenFalse, the detection triggers as soon as any listed anchor is inside.The implementation is a one-line dispatch (
np.allvsnp.any) on the existing per-anchor hit mask, so it adds no per-frame cost and preserves the existing out-of-bounds clipping.Test:
tests/detection/test_polygonzone.py::TestPolygonZoneTrigger::test_require_all_anchors_false_triggers_on_any_anchorcovers a box where only one of four anchors is inside, asserts the default rejects it, and assertsrequire_all_anchors=Falseaccepts it.