Add from_mmp_file function #198
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #198 +/- ##
==========================================
- Coverage 99.68% 95.91% -3.77%
==========================================
Files 11 11
Lines 634 661 +27
==========================================
+ Hits 632 634 +2
- Misses 2 27 +25 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
That's a great start @ivanvrig.
This PR has now been rebased to main, to bring it up to date with the refactored IO modules (see PR #194). I've left some comments which mostly have to do with aligning your function with the changes made in that PR.
Other, than that, here's a TODO list from our discussion today:
- add a valid MMPose .json file to our data repository, following the relevant instructions.
- use the above sample data to implement some unit tests for the new function (at least test what happens when loading a valid vs an invalid file).
- you may leave the json validator for a future PR. In fact I think you should, as it's not absolutely essential for this loader to function.
| [ | ||
| { | ||
| "frame_id": int, | ||
| "instances": [ | ||
| { | ||
| "keypoints": list[list[float]], | ||
| "keypoint_scores": list[float], | ||
| "bbox": list[float], | ||
| "bbox_score": float | ||
| }, | ||
| ... | ||
| ] | ||
| }, | ||
| ... | ||
| ] |
There was a problem hiding this comment.
This infor is useful, but better move it to the "Notes" section of the docstring, or better still add a reference to the corresponding MMPose documentation page where the format is describe. See from_sleap_file() for the syntax for "Notes" and "References".
movement/io/load_poses.py
Outdated
| valid_data = ValidPosesDataset( | ||
| position_array=tracks_array, | ||
| confidence_array=scores_array, | ||
| individual_names=[ | ||
| f"individual_{i}" for i in range(tracks_array.shape[1]) | ||
| ], | ||
| keypoint_names=keypoint_names, | ||
| fps=fps, | ||
| ) | ||
| ds = _from_valid_data(valid_data) |
There was a problem hiding this comment.
use the new from_numpy() function instead.
| ds = _from_valid_data(valid_data) | ||
|
|
||
| # Metadata | ||
| ds.attrs["source_software"] = "JSON" |
There was a problem hiding this comment.
I would call this "MMPose", as that is the software. json is just the file format and may be used by other software too.
|
Co-authored-by: Niko Sirmpilatze <[email protected]>
Co-authored-by: Niko Sirmpilatze <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Niko Sirmpilatze <[email protected]>
|
Description
What is this PR
Why is this PR needed?
This is the first step I have taken towards including I/O functionality for MMPose, as we discussed. The PR is unfinished as no tests have been written yet, I am mainly seeking feedback.
What does this PR do?
I have included a function from_mmp_file, which loads the data from the JSON file generated by the MMPose inferencer. I am mainly seeking feedback as it is a long function and might best be contributed by breaking it down into several (maybe loading as pandas df and then converting to xarray).
References
#175
How has this PR been tested?
It has not! Any feedback on how to properly write comprehensive tests for the movement package is welcome.
Is this a breaking change?
N/A
Does this PR require an update to the documentation?
Documentation should be updated to reflect the new function allowing for user input of MMPose Json files. I have included a lengthy docstring with the intention that it will explain functionality and expected input.
Checklist: