NeuralEnsemble / ephyviewer

Simple viewers for ephys signals, events, video and more
https://ephyviewer.readthedocs.io
MIT License
55 stars 14 forks source link

channel_indexes with AnalogSignalFromNeoRawIOSource and Neo 0.10.0 #156

Closed jpgill86 closed 3 years ago

jpgill86 commented 3 years ago

Hi @samuelgarcia,

I am starting to look more closely at #151. I have resisted upgrading to Neo 0.10.0 for months since I never fully understood the implications of switching to streams, and I never really looked at the code changes in #151. Now that an ephyviewer release is looming, I wanted to see if everything is OK, but I'm worried by what I've found.

In AnalogSignalFromNeoRawIOSource, you replaced channel_indexes with stream_index in the argument list for __init__. As far as I can tell, this means that channel indexes inside of a stream cannot be selected. In particular, get_chunk has no way to select a subset of channels within a stream.

Is this entirely intentional and necessary? Could we add an arugment to get_chunk (and maybe __init__ too) that lets a subset of channels be selected?

If this is not possible because of the way streams work in Neo 0.10.0, it is going to break a lot of my code. Perhaps you can help me understand whether I need to make fixes in the Neo reader that I built, AxographRawIO, or make changes elsewhere, like in neurotic.

With the release of Neo 0.10.0, I made no changes to AxographRawIO, apart from some generic changes you made. Presently, AxographRawIO always produces just one signal stream, since the stream ID '0' is hard-coded. This stream_id used to be called group_id. My understanding was that group_id, and stream_id still, are meant to differ when signals have different characteristics, like sampling rate, but otherwise it makes sense that signals share a group/stream ID. Since AxoGraph files only ever contain signals with uniform characteristics, I developed AxographRawIO with a hard-coded group/stream ID. I believe the example raw IO suggested these things too.

Now it seems that with the new changes to ephyviewer, I cannot select channels within a stream. For AxoGraph files, this means that all channels are always selected. Before, when channel_indexes existed in AnalogSignalFromNeoRawIOSource, I could select a subset of channels. Can we restore this capability?

jpgill86 commented 3 years ago

Also, unless we can make AnalogSignalFromNeoRawIOSource more backwards compatible, it may make sense to bump the version number to 1.5.0, rather than 1.4.1.

samuelgarcia commented 3 years ago

Hi jeffrey, I anderstand your skepticism about the "stream" concept in neo.rawio it is the same concept as previous "group" but more formal. For many reader it does not make sens at all and it is confusing for for some reader it really help and clarify the channel mapping (spikeglx, blackrock, openephys, ...)

I agree : in AnalogSignalFromNeoRawIOSource we loose sub channel selection for a given stream. I will add back the capability today. Note that channel_indexes will be now within stream and not global. I think it is beeter to put this channel select at init level like before rather than get_chunk otherwise we need to manage channel list everywhere.

And yes 1.5.0 make sens so.

jpgill86 commented 3 years ago

Closed by #157

Thanks again @samuelgarcia !