MRG, ENH: Refactor EGIMFF reader to use mffpy (Supersedes #12981)#13684
MRG, ENH: Refactor EGIMFF reader to use mffpy (Supersedes #12981)#13684PragnyaKhandelwal wants to merge 20 commits intomne-tools:mainfrom
Conversation
WIP: Use Mffpy for read_raw_egi [ci skip]
- more functional - annotate acquisition skips
There was a problem hiding this comment.
Hey @PragnyaKhandelwal If I am not mistaken, this looks incomplete. The motivation for using mffpy is to replace our current in-house MFF reader.
For the most part, it seems like this PR is still relying on MNE's own previously developed in-house code to parse the MFF files. e.g. I still see our internal functions such as _read_mff_header in use. In theory, mffpy should be doing that work for us!
In fact, I checked out this branch, and most of the tests in mne/io/egi/test will still 'pass' on my computer even when I don't have mffpy installed in my environment.
Others may feel free to disagree, but unfortunately I cannot approve this PR in its current state.
|
To me it looks AI generated too? @PragnyaKhandelwal could you tell us - to what extent did you used AI tools if any? |
e2f234c to
8887e02
Compare
for more information, see https://pre-commit.ci
…lwal/mne-python into fix-mffpy-refactor # Conflicts: # mne/io/egi/egimff.py
for more information, see https://pre-commit.ci
|
@scott-huberty I have completed the architectural refactor to fully decommission the legacy "in-house" reader logic. Key Changes:
Verification Proof: 1. Before mffpy installation (Strict warning & skipping): 2. After mffpy installation (All 24 tests passing): The PR is now green across the board and ready for your final review. |
…lwal/mne-python into fix-mffpy-refactor
for more information, see https://pre-commit.ci
…lwal/mne-python into fix-mffpy-refactor


Summary
This PR completes the migration of the EGI MFF reader to the
mffpybackend, resolving the architectural and numerical hurdles identified in previous attempts (specifically #12981).###Key Improvements & Technical Fixes:
_get_eeg_datato be "chunk-aware." It now correctly maps EEG/PNS tracks into pre-allocated buffers based on actual chunk dimensions rather than theoretical calculations, resolving theValueErrorbroadcaster mismatches.coordinates.xmloversensorLayout.xml. Corrected spatial scaling from cm to m and identified landmark names ("Nasion", "LPA", "RPA"), ensuring 137 -> 132 point alignment.np.arangeto integer-based sample offsets derived from chunk metadata.mffpy’s 9-decimal timestamps to preventdatetimeparsing failures and restoredutc_offsetsupport to ensuremeas_datecompatibility.logger.warn->warn) to comply with recent MNE-Python core standards.Validation Summary:
24/24 tests passed in

mne/io/egi/tests/. Verified locally on Python 3.11 withmffpy0.9.0 anddefusedxml.References:
read_raw_egi#12981 (Credits to @scott-huberty for the initial refactor draft).@scott-huberty @drammock I've addressed the primary blockers identified in #12981. This version provides a stable foundation for the upcoming release cycle.