[Bug]: Can't initialise `OpenEphysRecordingInterface` with more than one events folder

weiglszonja commented 2 months ago

What happened?

I tried to use NWB Guide to convert the openephys data from Constantinople lab, but received an error. The data is in OpenEphys binary format and has multiple streams (NIDAQ, AP, LF):

I can parse the stream names with OpenEphysBinaryRecordingInterface:

from neuroconv.datainterfaces import OpenEphysBinaryRecordingInterface

folder_path = "/Volumes/t7-ssd/GCP/Constantinople/J076_2023-12-06_13-24-28/Record Node 117"
print(OpenEphysBinaryRecordingInterface.get_stream_names(folder_path=folder_path))
>>>['Record Node 117#NI-DAQmx-114.PXI-6133', 'Record Node 117#Neuropix-PXI-119.ProbeA-AP', 'Record Node 117#Neuropix-PXI-119.ProbeA-LFP']

However, when I try to initialise the interface, I get an Exception that more than one events folder found (see traceback).

I haven't seen OpenEphys data with multiple streams, so I would appreciate any suggestions where to start fixing this. @CodyCBakerPhD (maybe @h-mayorquin ) FYI @bendichter

Steps to Reproduce

from neuroconv.datainterfaces import OpenEphysRecordingInterface

folder_path = "/Volumes/t7-ssd/GCP/Constantinople/J076_2023-12-06_13-24-28/Record Node 117"

interface = OpenEphysRecordingInterface(folder_path=folder_path)

Traceback

Traceback (most recent call last):
  File "/Users/weian/anaconda3/envs/constantinople-lab-to-nwb/lib/python3.11/site-packages/neuroconv/datainterfaces/ecephys/openephys/openephysbinarydatainterface.py", line 94, in __init__
    raise error
  File "/Users/weian/anaconda3/envs/constantinople-lab-to-nwb/lib/python3.11/site-packages/neuroconv/datainterfaces/ecephys/openephys/openephysbinarydatainterface.py", line 79, in __init__
    _open_with_pyopenephys(folder_path=folder_path)
  File "/Users/weian/anaconda3/envs/constantinople-lab-to-nwb/lib/python3.11/site-packages/neuroconv/datainterfaces/ecephys/openephys/openephysbinarydatainterface.py", line 18, in _open_with_pyopenephys
    pyopenephys_file = pyopenephys.File(foldername=folder_path)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/weian/anaconda3/envs/constantinople-lab-to-nwb/lib/python3.11/site-packages/pyopenephys/core.py", line 175, in __init__
    self._experiments.append(Experiment(op.join(self._absolute_foldername, rel_path), id, self))
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/weian/anaconda3/envs/constantinople-lab-to-nwb/lib/python3.11/site-packages/pyopenephys/core.py", line 263, in __init__
    Recording(op.join(self._absolute_foldername, rel_path), id, self, verbose=verbose)
  File "/Users/weian/anaconda3/envs/constantinople-lab-to-nwb/lib/python3.11/site-packages/pyopenephys/core.py", line 425, in __init__
    raise Exception("More than one events folder found!")
Exception: More than one events folder found!

Operating System

macOS

Python Executable

Conda

Python Version

3.11

Package Versions

No response

Code of Conduct

[X] I agree to follow this project's Code of Conduct
[X] Have you ensured this bug was not already reported?

bendichter commented 2 months ago

It looks like this might be an issue with pyopenephys, written by @alejoe91 (who is currently on vacation). @weiglszonja, could you please open a sister issue there and link it back to here?

bendichter commented 2 months ago

It looks like pyopenephys intentionally does not support binary data with > 1 events folder:

https://github.com/CINPLA/pyopenephys/blob/b81e29e852fe3badb0ef7beb9f763e80ead34029/pyopenephys/core.py#L416-L425

weiglszonja commented 2 months ago

It looks like pyopenephys intentionally does not support binary data with > 1 events folder:

https://github.com/CINPLA/pyopenephys/blob/b81e29e852fe3badb0ef7beb9f763e80ead34029/pyopenephys/core.py#L416-L425

Yeah right, but it seems like we're using it to get the session_start_time:

https://github.com/catalystneuro/neuroconv/blob/6a3046d187bbe81b07731857804df839ef505060/src/neuroconv/datainterfaces/ecephys/openephys/openephysbinarydatainterface.py#L120

so maybe there could be a workaround to still fetch that; or better if we could retrieve this from OpenEphysBinaryRecordingExtractor (but looking at raw_annotations suggests this can't be parsed from the extractor)

bendichter commented 2 months ago

Hmm, well first of all, let's try our best not to access private attributes. Private attributes can be removed without warning, which would break our code.

Second, yes, it does seem like there should be a more robust way to access this information. As a quick fix, let's just put this in a try/except, and raise an issue to handle this more robustly in the future.

weiglszonja commented 2 months ago

Would it be bold to parse the settings.xml file ourselves in NeuroConv for accessing the session_start_time? We're using OpenEphysBinaryRecordingExtractor from spikeinterface for writing the traces, it looks like in NeuroConv we really just care about accessing the DATE field from the settings.xml file which happens in pyopenephys: https://github.com/CINPLA/pyopenephys/blob/b81e29e852fe3badb0ef7beb9f763e80ead34029/pyopenephys/core.py#L324

bendichter commented 2 months ago

oh, yeah, let's just parse it ourselves. If that's really all we use pyopenephys for, that's overkill just to read and parse a date string

catalystneuro / neuroconv