-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add bbox centroid fix #303
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #303 +/- ##
=======================================
Coverage 99.77% 99.77%
=======================================
Files 14 14
Lines 883 884 +1
=======================================
+ Hits 881 882 +1
Misses 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with this to be merged as is, with 1 tiny suggestion.
The new test is a bit circular, you are right, but no better solutions come to my mind right now, and let's not overthink this.
Co-authored-by: Niko Sirmpilatze <[email protected]>
Quality Gate passedIssues Measures |
Description
What is this PR
Why is this PR needed?
When loading bboxes datasets from a VIA tracks file, we stored the coordinates of the top-left corner of each bbox in the dataset. This is inconsistent with the current description in the docs (which always refer to the centroid of the bboxes), and also somewhat inconsistent with the rest of the tools we support.
Fixes #302
What does this PR do?
Opinions on the tests are particularly welcome. I aimed to reduce the circularity but not sure if there is a better way.
How has this PR been tested?
Tests pass locally and in CI.
Is this a breaking change?
No.
Does this PR require an update to the documentation?
No, but docstrings of the relevant private functions have been updated.
Checklist: