search

catalystneuro / nwb-conversion-tools

Create NWB files by converting and combining neural data in proprietary formats and adding essential metadata.
https://nwb-conversion-tools.readthedocs.io/en/main/
BSD 3-Clause "New" or "Revised" License
25 stars 12 forks source link

Pre-select channels to load in `CEDRecordingInterface` #583

Open h-mayorquin opened 2 years ago

h-mayorquin commented 2 years ago

PR catalystneuro/nwb-conversion-tools#582 introduced the following change:

 def __init__(self, file_path: FilePathType, smrx_channel_ids: list, verbose: bool = True):
  changed to -> def __init__(self, file_path: FilePathType, verbose: bool = True):

@bendichter would like to see the feature re-implemented. Getting all the channels as a class method and provide functionality to load only those.

A solution: Looking at the implementation in the old spikeextractors I think we can just copy their old method (as it relies on sonpy) as the new class method and use spikeinterface channel slice machinery to confine the recording extractor to those channels.

We can have smrx_channel_ids = None to load all the channels and make that default behavior.

This however does not seem to improve IO efficiency in any way as the file has to be opened twice (as it was with spikeextractors).

What do you guys think?

CodyCBakerPhD commented 2 years ago

This machinery shouldn't be necessary any more, as I recall it was a limitation of the old extractor that it had to know which channels to load ahead of time, which was a source of some annoyance.

Since SpikeInterface loads them all naturally, if any subsetting of channels is desired in a given specific conversion project, they can modify the recording_extractor of the interface in whatever way they wish after it's initialized.

h-mayorquin commented 2 years ago

Relevant to this is the discussion in catalystneuro/neuroconv#8.