Add zoom_to_boxes, a zoom-in crop that keeps one box whole - #882
LisaDaVinchie wants to merge 1 commit into
Conversation
| first, last = 0, size - window | ||
| if low is not None: | ||
| first = max(first, math.ceil(high) - window) | ||
| last = min(last, math.floor(low)) | ||
| return random.randint(first, max(first, last)) |
There was a problem hiding this comment.
When an anchor extends outside the source image, _window_origin can return an origin beyond size - window. The standard detection loader can produce unclipped, out-of-frame boxes, and NumPy silently truncates slices outside the image. This can therefore produce a truncated or empty crop whose coordinates no longer represent a crop containing the anchor. Clamp or reject the anchor before calculating the legal origin range.
Knowledge Base Used: Data loading and augmentation
| area = (boxes[:, 2] - boxes[:, 0]) * (boxes[:, 3] - boxes[:, 1]) | ||
| visible = (clipped[:, 2] - clipped[:, 0]) * (clipped[:, 3] - clipped[:, 1]) | ||
| keep = (area > 0) & (visible >= min_visible * area) |
There was a problem hiding this comment.
Visibility fraction is unchecked
min_visible is documented as an area fraction but is not constrained to [0, 1]. A value at or below zero retains fully cropped-out boxes as zero-area labels, while a value above one drops even the wholly visible anchor. Validate the argument so the function preserves its stated label-validity contract.
| area = (boxes[:, 2] - boxes[:, 0]) * (boxes[:, 3] - boxes[:, 1]) | |
| visible = (clipped[:, 2] - clipped[:, 0]) * (clipped[:, 3] - clipped[:, 1]) | |
| keep = (area > 0) & (visible >= min_visible * area) | |
| if not 0.0 <= min_visible <= 1.0: | |
| raise ValueError(f"min_visible must be between 0 and 1. Got {min_visible}") | |
| area = (boxes[:, 2] - boxes[:, 0]) * (boxes[:, 3] - boxes[:, 1]) | |
| visible = (clipped[:, 2] - clipped[:, 0]) * (clipped[:, 3] - clipped[:, 1]) | |
| keep = (area > 0) & (visible >= min_visible * area) |
Knowledge Base Used: Augmentation pipeline
| return random.randint(first, max(first, last)) | ||
|
|
||
|
|
||
| def zoom_to_boxes(image, boxes, zoom_range=(1.0, 1.0), min_visible=0.6): |
There was a problem hiding this comment.
Development PR targets release
This development feature PR targets release, contrary to the repository directive that all development PRs target dev, never release. This repository requirement must be satisfied before merging.
Context Used: CLAUDE.md (source)
| return random.randint(first, max(first, last)) | ||
|
|
||
|
|
||
| def zoom_to_boxes(image, boxes, zoom_range=(1.0, 1.0), min_visible=0.6): |
There was a problem hiding this comment.
The required Code provenance section remains an unfilled template. The repository requires it to accurately describe the actual diff, and leaving it blank causes the provenance CI gate to fail. State whether this implementation is original or identify its upstream source, commit or version, and compatible license.
Context Used: CLAUDE.md (source)
| return random.randint(first, max(first, last)) | ||
|
|
||
|
|
||
| def zoom_to_boxes(image, boxes, zoom_range=(1.0, 1.0), min_visible=0.6): |
There was a problem hiding this comment.
The new augmentation is added only as a standalone geometry helper, with no recipe integration or coverage for YOLO9 or RF-DETR. This violates the repository directive that new features cover both flagship model families, so the requirement must be satisfied before merging.
Context Used: CLAUDE.md (source)
Knowledge Base Used: Augmentation pipeline
| return random.randint(first, max(first, last)) | ||
|
|
||
|
|
||
| def zoom_to_boxes(image, boxes, zoom_range=(1.0, 1.0), min_visible=0.6): |
There was a problem hiding this comment.
Shared change lacks justification
This adds behavior to the shared geometry layer without explaining why shared code is necessary or which model workflows it may affect. The repository requires shared-code changes to document their necessity and blast radius, so that rationale must be added before merging.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Code provenance
The PR is not ready to merge because out-of-frame anchors can create truncated or empty crops, provenance is missing, and explicit repository delivery requirements remain unmet.
Summary
Adds a standalone box-aware zoom-in crop to the shared augmentation geometry module.
Reviews (1) · Last reviewed commit: "Add zoom_to_boxes, a zoom-in crop that k..."