Open edeno opened 2 months ago
Attention: Patch coverage is 0%
with 57 lines
in your changes missing coverage. Please review.
Project coverage is 91.08%. Comparing base (
cf0a6b1
) to head (739c4d8
).:exclamation: Current head 739c4d8 differs from pull request most recent head 90c3287
Please upload reports for the commit 90c3287 to get more accurate results.
Files | Patch % | Lines |
---|---|---|
movement/io/nwb.py | 0.00% | 57 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks a lot for this @edeno 🤗 . We've been wanting to integrate with .nwb for a while, this is definitely on our roadmap. We'll take a look at the PR next week and get back to you with comments.
Just a heads up that we've just merged a PR introducing some new linting rules for docstrings. So it's probably a good idea to update from the main
branch (via rebase or a merge commit), to avoid any merge conflicts.
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
No data about Duplication
Hey @edeno, a recently merged PR introduced some breaking changes that affect the sample data and tests. You'd have to update (again) from the main branch (via rebase or a merge commit), for the CI to work and to avoid any merge conflicts. Sorry about that.
Also, do you need any help with making progress on this PR? Don't hesitate to poke me if you do.
Hi @niksirbi, that's fine. Happy to update.
Also thank you for your review. I haven't had time to work on this recently but I was intending to pick it up again when I have some more free time.
Hi @niksirbi, that's fine. Happy to update.
Also thank you for your review. I haven't had time to work on this recently but I was intending to pick it up again when I have some more free time.
No worries, this is not urgent. It's more important to get it right than to do it fast. Let me know when you get back to it (or if at any point you realise you can't find the time).
I've written a "developer guide" for adding new Input/Output functionalities to movement
, partly motivated by the ongoing work in this PR. I hope that it will help clarify the relevant code structure and make it easier to find things for contributors such as yourself @edeno. Have a read when you get back to working on this PR and let me know if anything is unclear.
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
No data about Duplication
@rly question for you: any plans to release ndx-pose 2.0 to pypi in the immediate future?
Description
What is this PR
Why is this PR needed? NWB is a standardized format for neuroscience data. As one of the goals of this package is to standardize output from pose estimation algorithms, it would be useful to output to NWB. In addition, importing from the ndx-pose extension would be useful as future algorithms could simply output to this format and then import into the common movement format.
What does this PR do? Exports
skeletons
andPoseEstimation
objects from thendx-pose
extension.References
Fix https://github.com/neuroinformatics-unit/movement/issues/23
How has this PR been tested?
Added export example in examples.
Is this a breaking change?
No.
Does this PR require an update to the documentation?
Have not updated documentation yet.
Checklist:
To-Do
Notes:
The
ndx-pose
package on pypi is not updated yet (@rly). The current code utilizes a pip install directly from https://github.com/rly/ndx-pose/ however the pre-commit hooks would not allow me to add a pip dependency for installing directly from the github repo.The current export code does not include anything about the training of the models.