Skip to content
Open
1 change: 1 addition & 0 deletions doc/changes/dev/13447.newcontrib.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve the error message raised by :func:`mne.viz.plot_topomap` (via :func:`mne.channels.layout._auto_topomap_coords`) when mixed 10–20 EEG electrode naming conventions are detected, by :newcontrib:`Aasma Gupta`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only reference public functions in changelogs

Suggested change
Improve the error message raised by :func:`mne.viz.plot_topomap` (via :func:`mne.channels.layout._auto_topomap_coords`) when mixed 10–20 EEG electrode naming conventions are detected, by :newcontrib:`Aasma Gupta`.
Improve the error message raised by :func:`mne.viz.plot_topomap` when mixed 10–20 EEG electrode naming conventions are detected, by :newcontrib:`Aasma Gupta`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AasmaGupta can you please integrate this suggestion?

1 change: 1 addition & 0 deletions doc/changes/names.inc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.. _Aaron Earle-Richardson: https://github.com/Aaronearlerichardson
.. _Aasma Gupta: https://github.com/AasmaGupta
.. _Abram Hindle: https://softwareprocess.es
.. _Adam Li: https://github.com/adam2392
.. _Adeline Fecker: https://github.com/adelinefecker
Expand Down
35 changes: 30 additions & 5 deletions mne/channels/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -971,11 +971,36 @@ def _auto_topomap_coords(info, picks, ignore_overlap, to_sphere, sphere):
for elec_i in squareform(dist < 1e-10).any(axis=0).nonzero()[0]
]

raise ValueError(
"The following electrodes have overlapping positions,"
" which causes problems during visualization:\n"
+ ", ".join(problematic_electrodes)
)
duplicates_1020 = {
"T3": "T7",
"T4": "T8",
"T5": "P7",
"T6": "P8",
}

names = set(problematic_electrodes)
duplicate_pairs = [
(old, new)
for old, new in duplicates_1020.items()
if old in names and new in names
]

if duplicate_pairs:
raise ValueError(
"Duplicate EEG electrode positions detected due to mixed 10-20 "
"naming conventions.\n"
"You appear to have both legacy (T3/T4/T5/T6) and modern "
"(T7/T8/P7/P8) electrode names present. The modern convention "
"(T7/T8/P7/P8) is recommended.\n\n"
"Please drop the legacy channels before plotting, for example:\n"
" inst.drop_channels(['T3', 'T4', 'T5', 'T6'])"
)
Comment on lines +974 to +997
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
duplicates_1020 = {
"T3": "T7",
"T4": "T8",
"T5": "P7",
"T6": "P8",
}
names = set(problematic_electrodes)
duplicate_pairs = [
(old, new)
for old, new in duplicates_1020.items()
if old in names and new in names
]
if duplicate_pairs:
raise ValueError(
"Duplicate EEG electrode positions detected due to mixed 10-20 "
"naming conventions.\n"
"You appear to have both legacy (T3/T4/T5/T6) and modern "
"(T7/T8/P7/P8) electrode names present. The modern convention "
"(T7/T8/P7/P8) is recommended.\n\n"
"Please drop the legacy channels before plotting, for example:\n"
" inst.drop_channels(['T3', 'T4', 'T5', 'T6'])"
)
# Check for duplicate 10-20 channels that are aliases for the same position
LEGACY_TO_MODERN_1020 = {"T3": "T7", "T4": "T8", "T5": "P7", "T6": "P8"}
names = set(problematic_electrodes)
conflicts = {
old: new
for old, new in LEGACY_TO_MODERN_1020.items()
if old in names and new in names
}
if conflicts:
overlap_info = ", ".join(f"{old}/{new}" for old, new in conflicts.items())
drop_list = ', '.join(repr(old) for old in conflicts)
raise ValueError(
"The following electrodes are aliases for the same physical location "
f"(10-20 vs 10-10): {overlap_info}\n. To fix this call "
f"`.drop_channels([{drop_list}])` on your Raw/Epochs/Evoked object."
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple notes on that suggestion:

  1. The error message is now a bit shorter and provides an actionable fix. The repr(old) just makes sure that e.g. 'T3' is printed instead of T3 so that the suggestion is valid Python code.
  2. I added a code comment at the beginning of this code block as a hint to future devs.
  3. condensed duplicates_1020 to a single line. IMO the new name is more descriptive.
  4. Make sure to remember to update the match=... In your test! Otherwise it will fail after incorporating this change.


else:
raise ValueError(
"The following electrodes have overlapping positions, which causes "
"problems during visualization:\n" + ", ".join(problematic_electrodes)
)

if to_sphere:
# translate to sphere origin, transform/flatten Z, translate back
Expand Down
19 changes: 19 additions & 0 deletions mne/channels/tests/test_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
assert_equal,
)

import mne
from mne import pick_info, pick_types
from mne._fiff.constants import FIFF
from mne._fiff.meas_info import _empty_info
Expand Down Expand Up @@ -186,6 +187,24 @@ def test_find_topomap_coords():
_find_topomap_coords(info, picks, **kwargs)


def test_duplicate_1020_electrode_names_error():
"""Ensure mixed 10–20 EEG naming conventions raise a clear error."""
montage = mne.channels.make_standard_montage("standard_1020")
data = np.random.randn(len(montage.ch_names))
info = mne.create_info(
ch_names=montage.ch_names,
sfreq=1000,
ch_types="eeg",
)
info.set_montage(montage)

with pytest.raises(
ValueError,
match="Duplicate EEG electrode positions detected due to mixed 10-20",
):
mne.viz.plot_topomap(data, pos=info)


def test_make_eeg_layout(tmp_path):
"""Test creation of EEG layout."""
lout_orig = read_layout(fname=lout_path / "test_raw.lout")
Expand Down