-
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
Fix and relax bboxes requirements #313
Conversation
71bf499
to
64eb1de
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #313 +/- ##
=======================================
Coverage 99.78% 99.78%
=======================================
Files 14 14
Lines 912 927 +15
=======================================
+ Hits 910 925 +15
Misses 2 2 ☔ View full report in Codecov by Sentry. |
Quality Gate passedIssues Measures |
5c27794
to
1413167
Compare
4736595
to
b5b7046
Compare
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.
Looks good @sfmig! That's some fixture factory extravaganza right there!
I only left very few clarifying comments/questions.
I also think you may have forgotten to update the docstring of the from_numpy()
functions. It currently says:
frame_array : np.ndarray, optional
Array of shape (n_frames, 1) containing the frame numbers for which
bounding boxes are defined. If None (default), frame numbers will
be assigned based on the first dimension of the ``position_array``,
starting from 0. If a specific array of frame numbers is provided,
these need to be consecutive integers.
I think they no longer need to be consecutive, right?
…ly monotonically increasing frame numbers)
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
8cd94c1
to
63cdb0e
Compare
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.
Just a few more comments
Quality Gate passedIssues Measures |
Description
What is this PR
Why is this PR needed?
The current implementation gives an error when loading a dataset with a single individual.
It also has some inconveniences that I realised were a bit too strict when I started using it more. Mainly:
frame_
, but often the filename is simply the frame number, so I removed this.from_numpy
it is up to the user to sort it.What does this PR do?
frame_
prefixtest_datasets_validators/test_bboxes_dataset_validator_frame_array
to check this behaves as expected.load_bboxes
testsReferences
Many of these issues were identified while working on #312
How has this PR been tested?
Tests pass locally and on CI.
Is this a breaking change?
No.
Does this PR require an update to the documentation?
The relevant docstrings have been updated.
Checklist: