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
307 stars 240 forks source link

Multiple issues with gettings durations for events from Open Ephys binary files #1437

Open johnmbarrett opened 2 months ago

johnmbarrett commented 2 months ago

Describe the bug OpenEphysBinaryRawIO._parse_header refuses to extract durations when the number of rising edges detected exceeds the number of falling edges, even though there is enough information to compute almost all durations in this case, and also incorrectly assumes that successive events always have alternating polarity.

To Reproduce 1) Download this OpenEphys dataset: https://drive.google.com/file/d/1Hr3-GFOpOkNc1hgXdVr5HTunchQm4enw/view?usp=drive_link 2) Unzip the dataset somewhere 3) Run the following code

from neo.rawio.openephysbinaryrawio import OpenEphysBinaryRawIO

folder = # fill in the folder you unzipped the data to

io = OpenEphysBinaryRawIO(folder)

io._parse_header()

timestamps, durations, labels = io._get_event_timestamps(0,0,0,None,None)

Expected behaviour The correct durations are returned for all events

Actual behaviour durations is None

Environment:

Additional context For the first problem, the check on line 245 of openephysbinaryrawio.py is unnecessarily stringent. It should be less than or equal, not strict equality.

Moreover, the logic in this section doesn't work when events from multiple binary channels are interleaved on the same event stream (which is allowed by the Open Ephys specification and happens for e.g. data from an Intan RHD2000 USB evaluation board), as it incorrectly assumes that events alternate between rising and falling. However, if multiple channels are interleaved, then two rising or two falling events from different channels can follow each other, and so the duration calculation on line 246 is wrong. In the example dataset, this happens for the 26th & 27th events, in which a falling edge on channel 2 follows a falling edge on channel 3. A better approach would be, for each rising edge, to look for the next falling edge with the same absolute state value (and either drop that event from the list or assign it a NaN duration if none is found, rather than Noneing out the entire durations array)

alejoe91 commented 1 month ago

@johnmbarrett would you have time to make a PR and propose a fix?