-
Notifications
You must be signed in to change notification settings - Fork 12
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
Reorder dimensions #351
Reorder dimensions #351
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #351 +/- ##
=======================================
Coverage 99.78% 99.78%
=======================================
Files 14 14
Lines 927 932 +5
=======================================
+ Hits 925 930 +5
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.
Thanks for undertaking this tricky surgery @lochhh, a PR that touches almost all files!
I couldn't find a fault with it and I'd say this is good to go, after resolving merge conflicts.
There is one thing I'm puzzled about, which you perhaps can help with.
In compute_norm()
, we have:
if "space" in data.dims:
validate_dims_coords(data, {"space": ["x", "y"]})
return xr.apply_ufunc(
np.linalg.norm,
data,
input_core_dims=[["space"]],
kwargs={"axis": -1},
)
kwargs={"axis": -1}
is passed onto linalg.norm
and I understood it as meaning the "norm along the space axis". You haven't updated this, so by my logic this should be broken now, but the passing tests indicate that it works as expected. What gives?
Quality Gate passedIssues Measures |
Thanks! Rebase is done now.
From xarray's
|
That's cool, thanks for clarifying! LGTM 🎉 |
Description
What is this PR
Why is this PR needed?
This PR closes #236.
What does this PR do?
This PR
poses
andbboxes
datasets as follows:poses
:("time", "individuals", "keypoints", "space")
→("time", "space", "keypoints", "individuals")
bboxes
:("time", "individuals", "space")
→("time", "space", "individuals")
assert_dataset
intest_load_poses
andtest_load_bboxes
by introducing a new helper classMovementDatasetAsserts
inconftest.py
that provides a commonvalid_dataset
function to assert poses and bboxes dataset validity in testsStrategy:
References
#236
How has this PR been tested?
Tests pass locally and on CI.
Does this PR require an update to the documentation?
Done.
Checklist: