Fix ImportError by implementing get_zones_for_point and Zone class fo…#83
Open
dyutishmaan-das wants to merge 4 commits into
Open
Fix ImportError by implementing get_zones_for_point and Zone class fo…#83dyutishmaan-das wants to merge 4 commits into
dyutishmaan-das wants to merge 4 commits into
Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds a Zone wrapper with point-in-polygon queries, extends DetectionSchema with ChangesZone Geometry and Detection Integration
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Detector
participant ZoneService
participant Renderer
participant SceneGraphBuilder
CLI->>Detector: start frame loop
Detector->>ZoneService: get_zones()
ZoneService-->>Detector: active_zones
Detector->>ZoneService: get_zones_for_point(cx,cy, zones=active_zones)
ZoneService-->>Detector: zones_present
Detector->>SceneGraphBuilder: add detection + zones
SceneGraphBuilder-->>CLI: serialized reasoning graph (every 30 frames)
Detector->>Renderer: draw_detections(detections, active_zones)
Renderer->>ZoneService: get_zones()
Renderer->>Renderer: render boxes using det.bbox.x1/y1/x2/y2 and zone.color_bgr
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
services/detection/zones.py (1)
56-67: ⚡ Quick winAvoid rebuilding all zones on every point query in the hot path.
get_zones_for_pointcallsget_zones()each invocation, which reallocatesZoneobjects repeatedly during per-detection processing. Accept an optional preloaded zone list (or prefetch once per frame) to reduce allocation and loader churn.Proposed refactor
-def get_zones_for_point(x: float, y: float) -> list[Zone]: +def get_zones_for_point(x: float, y: float, zones: list[Zone] | None = None) -> list[Zone]: @@ - for z in get_zones(): + for z in (zones if zones is not None else get_zones()): pts = z.as_array()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/detection/zones.py` around lines 56 - 67, get_zones_for_point currently calls get_zones() on every invocation causing repeated Zone allocations; change the function signature (get_zones_for_point) to accept an optional preloaded list parameter (e.g., zones: list[Zone] | None = None), then inside the function use the provided zones if not None else call get_zones(); keep the existing filtering logic (check pts length and cv2.pointPolygonTest) and update hot-path callers to pass a per-frame preloaded zones list to avoid rebuilding Zones each detection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/detection/detection.py`:
- Line 23: The code currently imports and uses the snapshot DEFAULT_ZONES for
rendering, which can drift from live membership; update rendering and any uses
of zones_present/overlays to call get_zones_for_point(...) or otherwise fetch
the live zones instead of referencing DEFAULT_ZONES. Locate references to
DEFAULT_ZONES in services/detection/detection.py (and any functions that compute
zones_present or draw overlays) and replace them so they query
get_zones_for_point(point) at render time, ensuring overlays and zones_present
use up-to-date zone data after config reloads.
In `@services/detection/zones.py`:
- Around line 24-37: The Zone constructor currently trusts data["polygon"] and
can crash later; update the Zone __init__ to validate and coerce polygon points:
iterate over data.get("polygon", []), accept only iterable points of length >=2,
coerce each coordinate to int (or drop the point on failure), and only keep
points with exactly two numeric ints; if fewer than 3 valid points, set
self.polygon = [] and set a flag like self.valid = False (True otherwise) and
ensure self.color_bgr is still computed as before; also make as_array() return
an empty np.ndarray when self.polygon is empty to avoid OpenCV errors.
---
Nitpick comments:
In `@services/detection/zones.py`:
- Around line 56-67: get_zones_for_point currently calls get_zones() on every
invocation causing repeated Zone allocations; change the function signature
(get_zones_for_point) to accept an optional preloaded list parameter (e.g.,
zones: list[Zone] | None = None), then inside the function use the provided
zones if not None else call get_zones(); keep the existing filtering logic
(check pts length and cv2.pointPolygonTest) and update hot-path callers to pass
a per-frame preloaded zones list to avoid rebuilding Zones each detection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91c9ba42-4930-4881-9c40-c0ffae24c274
📒 Files selected for processing (3)
libs/schemas/detection.pyservices/detection/detection.pyservices/detection/zones.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
services/detection/zones.py (1)
27-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard nullable YAML fields before iterating or stripping.
polygon: nullstill raises on Line 27, andcolor_hex: nullstill raises before the fallback on Line 36. Both cases can break module import instead of degrading to an invalid/defaulted zone.Proposed fix
- for point in data.get("polygon", []): + for point in data.get("polygon") or []: if isinstance(point, (list, tuple)) and len(point) == 2: try: self.polygon.append([int(point[0]), int(point[1])]) except (ValueError, TypeError): pass self.valid = len(self.polygon) >= 3 # Convert hex color to BGR for OpenCV - hex_color = data.get("color_hex", "`#FF0000`").lstrip("#") + hex_color = str(data.get("color_hex") or "`#FF0000`").lstrip("#")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/detection/zones.py` around lines 27 - 43, Guard against explicit nulls in YAML by treating None as missing: when iterating polygon use a falsy-coalesce (e.g., for point in (data.get("polygon") or [])) so None doesn't get iterated, and when reading color_hex check/coerce before calling lstrip (e.g., hex_src = data.get("color_hex") or "`#FF0000`" and only call hex_src.lstrip("#") or ensure isinstance(hex_src, str)); update the code that sets self.polygon, self.valid, hex_color, and self.color_bgr accordingly so null polygon/color_hex degrade to the empty polygon/default color fallback instead of raising.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@services/detection/zones.py`:
- Around line 27-43: Guard against explicit nulls in YAML by treating None as
missing: when iterating polygon use a falsy-coalesce (e.g., for point in
(data.get("polygon") or [])) so None doesn't get iterated, and when reading
color_hex check/coerce before calling lstrip (e.g., hex_src =
data.get("color_hex") or "`#FF0000`" and only call hex_src.lstrip("#") or ensure
isinstance(hex_src, str)); update the code that sets self.polygon, self.valid,
hex_color, and self.color_bgr accordingly so null polygon/color_hex degrade to
the empty polygon/default color fallback instead of raising.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18ce928d-c709-4fcd-86a5-034d70421444
📒 Files selected for processing (2)
services/detection/detection.pyservices/detection/zones.py
🚧 Files skipped from review as they are similar to previous changes (1)
- services/detection/detection.py
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
2 participants
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
This PR fixes the
ImportErrorcrash that occurs when runningmake demoordetection.py.The detection service was failing because it relied on
DEFAULT_ZONESandget_zones_for_point(), which were removed during the recent migration to the YAML-basedZoneConfigLoader.Changes Made
Zoneclass inzones.pyto wrap the raw YAML dictionaries, providing the.nameand.as_array()attributes expected by downstream code.get_zones_for_point(x, y)usingcv2.pointPolygonTestto accurately determine which zone a detected object is in.centerandzones_presentfields to the PydanticDetectionSchema.SceneGraphBuilderandbuild_reasoning_promptimports indetection.py.BoundingBoxwas instantiated as a list instead of a Pydantic object.Fixes #81
GSSoC 2026 Contribution
Summary by CodeRabbit
New Features
Refactor