Skip to content
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

Add NWBIO #796

Merged
merged 90 commits into from
Jul 22, 2021
Merged

Add NWBIO #796

merged 90 commits into from
Jul 22, 2021

Conversation

apdavison
Copy link
Member

@apdavison apdavison commented Mar 6, 2020

This PR adds NWBIO, an IO module for reading and writing the Neurodata Without Borders format (cf #221).

This is a work in progress, which doesn't support all possible contents of an NWB file, but I think it is good enough for people to try it, and let us know (by commenting here) what doesn't work, or could be done better. Many thanks to @legouee for her work on this.

TODO

  • write Block, Segment, Epoch, Event, AnalogSignal, IrregularlySampledSignal, SpikeTrain using base TimeSeries
  • write Unit, ChannelIndex (or whatever replaces them)
  • write ImageSequence and related
  • allow user to choose where to write data (e.g. "acquisition", "stimulus", "analysis") by providing guiding annotations
  • allow user to write subclasses of TimeSeries by providing guiding annotations
  • read "acquisition" and "stimulus" groups
  • read metadata associated with processing modules and make it available through annotations
  • support array annotations

Usage example: https://github.com/apdavison/python-neo/blob/nwb2/examples/NWB-Allen-Institute-Example.ipynb

@JuliaSprenger
Copy link
Member

@apdavison @legouee I think we can merge this PR soon to have a first version of the NWBIO in the next release. The most important change requests for this would be the incorporation of some testfiles also on gin (if you have some, as we can then tag the release to a fixed version of the test files), the incorporation of the BaseTests and some completion that is still lacking on the docstrings. Also I am not sure about some of the discussions in this thread form the end of last year, if there are outdated, could you resolve them?

neo/io/nwbio.py Outdated Show resolved Hide resolved
neo/io/nwbio.py Outdated Show resolved Hide resolved
neo/io/nwbio.py Outdated Show resolved Hide resolved
neo/io/nwbio.py Outdated Show resolved Hide resolved
neo/io/nwbio.py Outdated Show resolved Hide resolved
neo/io/nwbio.py Outdated Show resolved Hide resolved
neo/io/nwbio.py Outdated Show resolved Hide resolved
apdavison and others added 4 commits July 8, 2021 13:48
Implement suggestions from review of PR for nwb2 branch
@JuliaSprenger
Copy link
Member

@legouee @apdavison The test file is now on gin. Can you include it in the set of files used by the unittests and also include the common rawio tests by inheriting from BaseTestIO?

@samuelgarcia
Copy link
Contributor

@JuliaSprenger : since it now use BaseTestIO for tests, I think we can merge no ?

@samuelgarcia
Copy link
Contributor

There are some warning in test_nwb.py
I guess we can live with then for a while.

@JuliaSprenger JuliaSprenger mentioned this pull request Jul 22, 2021
12 tasks
@JuliaSprenger
Copy link
Member

@apdavison @legouee I cleaned all open comments, moved unresolved ones to #1013 for the future. I fixed the layout to be pep8 compatible and fixed some docstrings / typos. Under the assumption that you don't plan to include apdavison#6 here any more I would be ok to merge the current version and fix the remaining issues listed in #1013 in the next release.

@JuliaSprenger JuliaSprenger merged commit 89eaaf7 into NeuralEnsemble:master Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants