Skip to content

Fix cleaning of channel names which can create duplicate for non vectorview or CTF systems#12489

Merged
larsoner merged 5 commits intomne-tools:mainfrom
mscheltienne:fix
Mar 11, 2024
Merged

Fix cleaning of channel names which can create duplicate for non vectorview or CTF systems#12489
larsoner merged 5 commits intomne-tools:mainfrom
mscheltienne:fix

Conversation

@mscheltienne
Copy link
Copy Markdown
Member

@mscheltienne mscheltienne commented Mar 11, 2024

Reported on the forum here: https://mne.discourse.group/t/error-when-trying-to-plot-projectors-ssp/8456

MWE:

from pathlib import Path

from mne import read_proj
from mne.io import read_info
from mne.viz import plot_projs_topomap


directory = Path.home() / "Downloads"
info = read_info(directory / "test-info.fif")
projs = read_proj(directory / "test-proj.fif")
plot_projs_topomap(projs, colorbar=True, vlim="joint", info=info)

Dataset: test-projs.zip

I'm not sure if this is the best fix as I'm not familiar with the channel name format of old vectorview system and of CTF system; thus I'm not certain about the intended purpose of _clean_names.

@mscheltienne mscheltienne changed the title Fix Fix cleaning of channel names which can create duplicate for non vectorview or CTF systems Mar 11, 2024
@larsoner
Copy link
Copy Markdown
Member

larsoner commented Mar 11, 2024

I'm not sure if this is the best fix as I'm not familiar with the channel name format of old vectorview system and of CTF system; thus I'm not certain about the intended purpose of _clean_names.

IIRC it started out to take old-style names like in sample of MEG 0111 and turn them into new-style names used for the last 10 (20?) years like MEG0111. It might have evolved to more than that but that was the original purpose.

Can you add a little test? For example you could load an existing CTF dataset and .rename_channels to the problematic names, then run whatever command(s) caused you problems

@mscheltienne
Copy link
Copy Markdown
Member Author

mscheltienne commented Mar 11, 2024 •
edited
Loading

I'm not convinced of the value of a test loading a CTF dataset and there is already test_rawctf_clean_names (haven't looked in details to that what this test does).
The issue at hand is that this user has an OPM dataset with channel names which get caught in the before_dash condition of _clean_names and yields duplicate channels. I added a test copy/pasting 4 of the channel names from this user.

@larsoner larsoner enabled auto-merge (squash) March 11, 2024 18:57
@larsoner
Copy link
Copy Markdown
Member

larsoner commented Mar 11, 2024

Ahh makes sense I misread the CTF stuff, marking for merge when green -- thanks in advance @mscheltienne !

@larsoner larsoner merged commit 9006789 into mne-tools:main Mar 11, 2024
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
@mscheltienne @snwnde
Fix cleaning of channel names which can create duplicate for non vect…
055a0d6 
…orview or CTF systems (mne-tools#12489)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@larsoner larsoner Awaiting requested review from larsoner larsoner is a code owner

@drammock drammock Awaiting requested review from drammock drammock is a code owner

@agramfort agramfort Awaiting requested review from agramfort agramfort is a code owner

@dengemann dengemann Awaiting requested review from dengemann

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mscheltienne @larsoner