FIX: add recover_epochs fallback for EGI .mff epoch metadata mismatch#13992
FIX: add recover_epochs fallback for EGI .mff epoch metadata mismatch#13992kveni12 wants to merge 4 commits into
Conversation
…mne-tools#13951) When epochs.xml sample counts don't match the binary signal data (e.g. improperly closed recordings, PSG/PNS async writes, impedance pauses), read_raw_egi now accepts recover_epochs=True to rebuild epoch boundaries from the actual binary block structure instead of raising RuntimeError. Default behavior (recover_epochs=False) is unchanged.
|
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴 |
for more information, see https://pre-commit.ci
|
Seems like a reasonable idea to me, thoughts @PragnyaKhandelwal since you're interacting with this code a bit now? |
| @requires_testing_data | ||
| def test_recover_epochs_raises_by_default(tmp_path): | ||
| """Test that mismatched epoch metadata raises RuntimeError by default.""" | ||
| import re as _re |
|
|
||
| recover_epochs : bool | ||
| If True and an MFF file has inconsistent epoch metadata (e.g., from an | ||
| improperly closed recording or interrupted acquisition), attempt to recover | ||
| the epoch structure from the binary signal data instead of raising an error. | ||
| A :class:`RuntimeWarning` is emitted when recovery is performed. | ||
| Default is False. | ||
|
|
||
| .. versionadded:: 1.13.0 | ||
|
|
There was a problem hiding this comment.
Style CIs will complain about these extra newlines
| recover_epochs : bool | |
| If True and an MFF file has inconsistent epoch metadata (e.g., from an | |
| improperly closed recording or interrupted acquisition), attempt to recover | |
| the epoch structure from the binary signal data instead of raising an error. | |
| A :class:`RuntimeWarning` is emitted when recovery is performed. | |
| Default is False. | |
| .. versionadded:: 1.13.0 | |
| recover_epochs : bool | |
| If True and an MFF file has inconsistent epoch metadata (e.g., from an | |
| improperly closed recording or interrupted acquisition), attempt to recover | |
| the epoch structure from the binary signal data instead of raising an error. | |
| A :class:`RuntimeWarning` is emitted when recovery is performed. | |
| Default is False. | |
| .. versionadded:: 1.13.0 |
|
|
||
|
|
||
| def _read_mff_header(filepath): | ||
| def _read_mff_header(filepath, recover_epochs=False): |
There was a problem hiding this comment.
| def _read_mff_header(filepath, recover_epochs=False): | |
| def _read_mff_header(filepath, *, recover_epochs=False): |
Looks good to me! The approach is clean and the default One thing worth flagging for the future: the recovery fallback relies on |
|
Okay @kveni12 you should be good to address the comments I made above ☝️ and push . Let me know when you do and I should be able to look and mark for merge-when-green! |
3 participants
Summary
Fixes #13951.
mne.io.read_raw_egicrashes withRuntimeErrorwhen reading certain EGI.mfffiles whoseepochs.xmlsample counts don't match the actual binary data. This happens with improperly closed recordings, PSG/PNS async writes, or impedance check pauses that create irregular fractional epochs.This PR adds a
recover_epochs=Falsekeyword argument toread_raw_egi. WhenTrue, instead of raising, the epoch boundaries are rebuilt from the actual binary block structure (signal1.bin) and aRuntimeWarningis emitted.Changes
mne/io/egi/egimff.py:_read_mff_headerand_read_headeracceptrecover_epochs; on mismatch, recover fromsignal_blocks["samples_block"]instead of raisingmne/io/egi/egi.py:read_raw_egiexposesrecover_epochs=Falsewith docstringmne/io/egi/tests/test_egi.py: two new tests — one asserts the default still raises, one assertsrecover_epochs=Trueloads successfully and emitsRuntimeWarningTest plan
test_recover_epochs_raises_by_default— corruptedepochs.xmlwithrecover_epochs=FalseraisesRuntimeErrortest_recover_epochs_succeeds_with_flag— same file withrecover_epochs=Trueloads and emitsRuntimeWarning