search

NeuralEnsemble / python-neo

Neo is a package for representing electrophysiology data in Python, together with support for reading a wide range of neurophysiology file formats
http://neo.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
325 stars 248 forks source link

EDF blocks access when io is opened. #1557

Open h-mayorquin opened 2 months ago

h-mayorquin commented 2 months ago

This is coming from here in Spikeinterface:

https://github.com/SpikeInterface/spikeinterface/issues/1228

Here, basically this format can't only be opened once per file-system.

from pathlib import Path

from spikeinterface.extractors import EDFRecordingExtractor
from spikeinterface.core.base import _load_extractor_from_dict

file_path = Path("/home/heberto/neuroconv_testing_data/ephy_testing_data/edf/edf+C.edf")

recorder = EDFRecordingExtractor(file_path=file_path)
recorder_dict = recorder.to_dict()
recorder_from_dict = _load_extractor_from_dict(recorder_dict)

  File "/home/heberto/miniconda3/envs/spikeinterface_env/lib/python3.10/site-packages/neo/rawio/edfrawio.py", line 70, in _parse_header
    self.edf_reader = EdfReader(self.filename)
  File "pyedflib/_extensions/_pyedflib.pyx", line 146, in pyedflib._extensions._pyedflib.CyEdfReader.__init__
  File "pyedflib/_extensions/_pyedflib.pyx", line 209, in pyedflib._extensions._pyedflib.CyEdfReader.open
  File "pyedflib/_extensions/_pyedflib.pyx", line 181, in pyedflib._extensions._pyedflib.CyEdfReader.check_open_ok
OSError: /home/heberto/neuroconv_testing_data/ephy_testing_data/edf/edf+C.edf: file has already been opened
zm711 commented 2 months ago

Wasn't this a long term issue. I have no clue how to fix. Since you have some data from somewhere are you okay trying to make a fix?

h-mayorquin commented 2 months ago

Yeah, unfortunately, the fix would be to redo the IO entirely as we use a library for this one and is blocking. I don't think it would be too hard but I would be more weary about testing it.

h-mayorquin commented 2 months ago

Mmm now that I think about it, we could use MNE. That should be easier.

zm711 commented 2 months ago

Yeah if it's possible. That'd be cool.

h-mayorquin commented 1 month ago

@zm711 can you re-open this? I don't think this is fully solved fully yet and the IO is just restricted to get_analogsignal... but it is still blocked.

The solution will be to either use mne or roll this into our own memmaps.

zm711 commented 1 month ago

I think it autoclosed when I merged. happy to reopen.

zm711 commented 1 month ago

Should I also modify the milestone to future in this case?

h-mayorquin commented 1 month ago

Yes, please do.