Convert predictions to cliplabels.json using movement#45
Conversation
lochhh
left a comment
There was a problem hiding this comment.
Thanks @sfmig , I think the decision to use movement for loading predictions and sleap-io for loading annotations makes a lot of sense! Most suggestions are trivial, but the main comments are related to the lack of error-handling in predictions_to_poseinterface and tests that exercise the function's various possible behaviours.
|
FYI I've now merged neuroinformatics-unit/movement#920 into Once we release, this PR can be rebased to directly use |
|
@sfmig when you get the chance to get back to this, you may find it useful to cherry-pick the following commits from PR #49:
|
predictions_to_poseinterface produces labels for a full video, not a clip. Per the spec, this intermediate file should use the _videolabels suffix. Rename _convert_movement_ds_to_cliplabels accordingly and update the output filename and all references in tests.
Co-authored-by: Chang Huan Lo <changhuanlo@yahoo.com>
6baa5b3 to
04b4134
Compare
|
Thanks @lochhh and apologies for the delay. I think all comments are addressed, will re-request now. |
lochhh
left a comment
There was a problem hiding this comment.
Thanks again @sfmig for addressing all comments! Pre-approving as there are only two minor things: duplicate movement dependency in pyproject.toml, suggestion to remove assert function calls (wiring will be tested in integration tests).
Description
What is this PR
Why is this PR needed?
To convert prediction files supported by
movementtocliplabels.jsonfiles, that we can clip later when extracting video clips.What does this PR do?
It adds a
predictions_to_poseinterfacefunction that reads prediction files as movement datasets and exports the trajectories as acliplabels.jsonfile, defined for the full range of frames in the prediction file.References
\
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.
Checklist: