Invalidate evaluation metrics on annotation bounding-box update#1628
Invalidate evaluation metrics on annotation bounding-box update#1628MalteEbner wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughBounding box updates now delete evaluation annotation and sample metrics associated with the annotation before saving new coordinates. Tests create both metric types, perform the update, and verify their removal. ChangesBounding box metric invalidation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce2372ac44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lightly_studio/src/lightly_studio/resolvers/annotation_resolver/update_bounding_box.py (1)
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
delete_annotationas a module heredelete_evaluation_metricsis a direct function import from a non-allowlisted module. Importdelete_annotationinstead and calldelete_annotation.delete_evaluation_metrics(...)to match the existing module import pattern.🤖 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 `@lightly_studio/src/lightly_studio/resolvers/annotation_resolver/update_bounding_box.py` around lines 12 - 14, The direct import of delete_evaluation_metrics violates the module import pattern. In update_bounding_box.py, import the delete_annotation module instead and update all references to call delete_annotation.delete_evaluation_metrics(...).Source: Coding guidelines
🤖 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.
Nitpick comments:
In
`@lightly_studio/src/lightly_studio/resolvers/annotation_resolver/update_bounding_box.py`:
- Around line 12-14: The direct import of delete_evaluation_metrics violates the
module import pattern. In update_bounding_box.py, import the delete_annotation
module instead and update all references to call
delete_annotation.delete_evaluation_metrics(...).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cc9e6e30-3b15-4482-9271-46584054448b
📒 Files selected for processing (2)
lightly_studio/src/lightly_studio/resolvers/annotation_resolver/update_bounding_box.pylightly_studio/tests/resolvers/annotations/test_annotations_update_bounding_box.py
ce2372a to
8d8be61
Compare
Bounding-box updates mutate the object-detection detail row in place and previously left the precomputed evaluation metrics (evaluation_annotation_metric, evaluation_sample_metric) stale. Reuse the existing delete_evaluation_metrics() helper so the affected rows are invalidated, matching the delete path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8d8be61 to
005f60c
Compare
|
/review |
1 participant
What has changed and why?
Per-annotation model evaluation results are precomputed. When updating the bbox, they did not update. This PR deletes them, because they are invalid.
Split out of #1612; the segmentation-mask half follows in a separate PR.
Design decision: Delete them instead of marking them as stale
Delete the invalid rows. This is easiest and automatically makes all consumers work.
Marking stale would only be needed if we would want to recompute only the invalid evaluation metrics instead of directly all metrics. I don't see a need for that.
How has it been tested?
new unittests
Did you update CHANGELOG.md?
Summary by CodeRabbit
Bug Fixes
Tests