buffer description api in neorawio and xarray reference API bridge#1513
buffer description api in neorawio and xarray reference API bridge#1513zm711 merged 31 commits intoNeuralEnsemble:masterfrom
Conversation
|
Looks good! |
…_buffer_description_api=True This should also solve the memmap and memmory leak problem.
…_buffer_description_api=True This should also solve the memmap and memmory leak problem.
Co-authored-by: Zach McKenzie <92116279+zm711@users.noreply.github.com>
h-mayorquin
left a comment
h-mayorquin
left a comment
There was a problem hiding this comment.
I did a first reading. I think that this PR and the future documentation would benefit greatly if we have the schema of the buffer description somewhere. It does not have to be a formal schema (although that would be great) it could be just a description on the documentation or a python data class with types. Something that I can reference to see what should I expect to fill when I am doing a buffer like this.
| For reading analog signals **neo.rawio** has 2 important concepts: | ||
|
|
||
| 1. The **signal_stream** : it is a group of channels that can be read together using :func:`get_analog_signal_chunk()`. | ||
| This group of channels is guaranteed to have the same sampling rate, and the same duration per segment. |
There was a problem hiding this comment.
Now that we have logical channels, this should be same units as well, right?
There was a problem hiding this comment.
At the moment not yet. The API do not enforce and ensure this.
See this https://github.com/NeuralEnsemble/python-neo/blob/master/neo/rawio/baserawio.py#L118
Ideally units should be add but some IO are mixing maybe the units in the same stream.
There was a problem hiding this comment.
Can we add it as an ideal? Or would you rather first do the changes and then change it here?
I can close this for example:
There was a problem hiding this comment.
There are some tricky cases where the units is manually done by the user for some formats.
not sure but maybe spike2 is in that case.
What should we do ? split all groups of same units in separate streams ? this could be an good idea but we need first a deeper check.
There was a problem hiding this comment.
Agree. I think that the concept of stream should include same unit across the stream but this we will need to solve in a case by case basis (the deeper check that you mention).
zm711
left a comment
zm711
left a comment
There was a problem hiding this comment.
couple more comments from me too.
|
Merci beaucoup @zm711 and @h-mayorquin for the review |
samuelgarcia
changed the title
|
@zm711 @h-mayorquin @alejoe91 |
|
yeah!! |
5 participants
Given the idea of @bendichter and this gist
This is an implementation of "buffer_description_api" for reading analogsignal chunk.
This 100% backward compatible.
The idea/goal is:
See also kerchunk