Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
I/O support for the ndx-pose NWB extension #166
I/O support for the ndx-pose NWB extension #166
Changes from 11 commits
bd12d04
c5319b9
d82fe30
d889105
72aea47
12bf83f
742bf86
a06d485
739c4d8
aef9b0c
ce28f90
b9599a9
2491cf6
58bef93
3ab9aa4
910ce90
3f9a53b
3cd991d
3aa1b11
e56cf6d
99a90c1
02b9975
a2ac053
84a495d
90c3287
e9e1cef
3ccd71c
f906cd5
d35d9c2
e5726d4
53f505b
f1d480d
4b162cf
4b887be
96ee7ba
2f2625d
1c7c2e3
4191ae8
4202ff6
3188b0b
4908040
da43e87
9d34939
56a6672
b37b2c6
f7d48ce
0606add
dbf804a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Similarly for this function, it's public, so a full docstring is needed.
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.
There seems to be an assymetry between the two converter functions:
convert_nwb_to_movement
reads nwb files from disk, whileconvert_movement_to_nwb
writes to the NWB file handlers, without actually writing to disk.To be consistent with our other loading/saving functions, I'd prefer to have functions to read from nwb file(s) on disk to movement ds, and functions for doing the exact opposite, i.e. write from movement ds to nwb file(s) on disk.
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.
Also, in our other IO functions we accept both
str
andpathlib.Path
as file paths.In this particular fuction it would make sense to accept either a single path (
Union[Path, str]
) (which would work for single-subject datasets), or a list of such paths.You could alro re-use our
movement.io.validators.ValidFile
validator for checking the paths, either directly, or via the_validate_file_path
utility that can be found inmovement.io.save_poses
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.
I think my reasoning for not writing from movement dataset to NWB file on disk is because there are often times where you'd like to add other things to the NWB file before writing to disk.