Repository navigation
FIX: Fix montage handling for EOG channels in EEGLAB reader - #14396
Merged
larsoner merged 4 commits intoOct 12, 2026
Merged
Conversation
sappelhoff
requested review from
agramfort,
drammock and
larsoner
as code owners
October 9, 2026 19:28
sappelhoff
force-pushed
the
fix-10505-eeglab-eog-montage
branch
from
October 9, 2026 19:49
d4584a1 to
e6e9833
Compare
larsoner
reviewed
Oct 10, 2026
| @@ -0,0 +1 @@ | |||
| Fix bug where :func:`mne.io.read_raw_eeglab` and :func:`mne.io.read_epochs_eeglab` raised a false warning when setting montage with EOG channels, by `Stefan Appelhoff`_. | |||
Member
There was a problem hiding this comment.
Suggested change
| Fix bug where :func:`mne.io.read_raw_eeglab` and :func:`mne.io.read_epochs_eeglab` raised a false warning when setting montage with EOG channels, by `Stefan Appelhoff`_. | |
| Fix bug where :func:`mne.io.read_raw_eeglab` and :func:`mne.read_epochs_eeglab` raised a false warning when setting montage with EOG channels, by `Stefan Appelhoff`_. |
| ) | ||
| _ensure_fiducials_head(eeg_montage.dig) | ||
| else: | ||
| eeg_montage = None |
Member
There was a problem hiding this comment.
Any chance to DRY the raw and epochs code? A quick look suggests they are very similar
Member
Author
There was a problem hiding this comment.
RawEEGLAB and EpochsEEGLAB already share _get_info() and _set_dig_montage_in_init() (their __init__ methods differ because BaseRaw and BaseEpochs have separate initialization paths for streaming vs. 3D trial arrays).
However:
- In
eeglab.py, the montage was previously being constructed twice (created once in_get_montage_information(), then deconstructed and recreated in_get_info()). I passedeogdirectly to_get_montage_information()so channel types are set up-front, building the montage only once and eliminating the duplicate reconstruction block in_get_info(). - In
test_eeglab.py, I parametrizedtest_eeglab_eog_montage()across bothread_raw_eeglabandread_epochs_eeglab, DRYing the test logic.
When reading EEGLAB files, channels of non-EEG types (such as EOG, ECG, STIM, or MISC) should not be included in the DigMontage. In MNE-Python, inst.set_montage() only applies positions to EEG-like channels and warns when non-EEG channels are present in montage.ch_pos, recommending to omit them when creating the montage. This commit: - Updates _get_montage_information() to take the eog parameter and designate EOG channels in ch_types before filtering montage_ch_pos, ensuring the montage is created only once without non-EEG channels. - Updates ch_types in _get_info() for channels matching the eog parameter. - Replaces manual NaN padding of missing channels in _set_dig_montage_in_init() with self.set_montage(montage, on_missing='ignore'), avoiding dummy NaN points in DigMontage. Fixes mne-tools#10505
Add parametrized test_eeglab_eog_montage() to verify that: 1. read_raw_eeglab() with eog parameter on files with channel positions does not raise RuntimeWarning regarding setting positions of EOG channels, correctly sets channel types to 'eog', and omits EOG channels from the returned montage. 2. read_epochs_eeglab() with eog parameter behaves similarly without warnings. Also allow **kwargs in get_bad_information() in test_fidsposition_information() to accept additional keyword arguments. References mne-tools#10505
Add towncrier changelog fragment documenting the bugfix for mne-tools#10505.
sappelhoff
force-pushed
the
fix-10505-eeglab-eog-montage
branch
from
October 10, 2026 17:50
e6e9833 to
48e0e7b
Compare
larsoner
reviewed
Oct 12, 2026
Comment on lines
+972
to
+975
| with warnings.catch_warnings(): | ||
| warnings.filterwarnings("error", message=".*setting position.*") | ||
| warnings.filterwarnings("ignore", message=ignore_warn) | ||
| inst = reader(fname, eog=eog) |
Member
There was a problem hiding this comment.
You shouldn't need to error filter as it's our default in tests. So I think this can more simply be a single
with pytest.warns(RuntimeWarning, match="setting position")
or similar
Member
Author
There was a problem hiding this comment.
thanks, I pruned the unneeded bits
Member
|
Thanks @sappelhoff ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
eogparam into account when setting montage #10505Gemini 3.8 flash helped with writing the code for this one
Summary
When reading EEGLAB files with channel positions and EOG channels:
_get_montage_information()only adds channels whose type is in("eeg", "seeg", "dbs", "ecog")toDigMontage._get_info()prunes any channels designated via theeogparameter fromeeg_montage._set_dig_montage_in_init()callsself.set_montage(montage, on_missing="ignore")instead of padding missing channels with dummy NaN coordinates.