Skip to content

MRG: Automatically selects EEG channels when plotting bridged electrodes on topomap #10753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
drammock merged 8 commits into from
Jun 14, 2022
Merged

MRG: Automatically selects EEG channels when plotting bridged electrodes on topomap #10753

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
660a591
add support for instance with more than 1 channel type
mscheltienne Jun 13, 2022
e80da7c
add entry to changelog
mscheltienne Jun 13, 2022
31e1fdf
add notes [skip ci]
mscheltienne Jun 13, 2022
a31e981
update gh id
mscheltienne Jun 13, 2022
0cbd750
fix doc [skip azp] [skip actions]
mscheltienne Jun 14, 2022
96c6e67
fix sphere issue
mscheltienne Jun 14, 2022
ba2dd3c
better
mscheltienne Jun 14, 2022
d555f75
Make docstring more concise & add See Also [skip azp][skip actions]
hoechenberger Jun 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/changes/latest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ Bugs

- In :class:`mne.Report`, some figures would have an undesired border added to the edges; this has now been resolved (:gh:`10730` by `Richard Höchenberger`_)

- Fix selection of EEG channels and selected sphere when plotting bridged electrodes with :func:`mne.viz.plot_bridged_electrodes` (:gh:`10753` by `Mathieu Scheltienne`_)

API and behavior changes
~~~~~~~~~~~~~~~~~~~~~~~~
- When creating BEM surfaces via :func:`mne.bem.make_watershed_bem` and :func:`mne.bem.make_flash_bem`, the ``copy`` parameter now defaults to ``True``. This means that instead of creating symbolic links inside the FreeSurfer subject's ``bem`` folder, we now create "actual" files. This should avoid troubles when sharing files across different operating systems and file systems (:gh:`10531` by `Richard Höchenberger`_)
Expand Down
10 changes: 9 additions & 1 deletion mne/viz/tests/test_topomap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@
from mne.io.proj import make_eeg_average_ref_proj, Projection
from mne.io import read_raw_fif, read_info, RawArray
from mne.io.constants import FIFF
from mne.io.pick import pick_info, channel_indices_by_type
from mne.io.pick import pick_info, channel_indices_by_type, _picks_to_idx
from mne.io.compensator import get_current_comp
from mne.channels import (read_layout, make_dig_montage, make_standard_montage,
find_ch_adjacency)
from mne.datasets import testing
from mne.preprocessing import compute_bridged_electrodes
from mne.time_frequency.tfr import AverageTFR

from mne.viz import plot_evoked_topomap, plot_projs_topomap, topomap
Expand Down Expand Up @@ -730,6 +731,13 @@ def test_plot_bridged_electrodes():
with pytest.raises(RuntimeError, match='Expected'):
plot_bridged_electrodes(info, bridged_idx, np.zeros((5, 6, 7)))

# test with multiple channel types
raw = read_raw_fif(raw_fname, preload=True)
picks = _picks_to_idx(raw.info, "eeg")
raw._data[picks[0]] = raw._data[picks[1]] # artificially bridge electrodes
bridged_idx, ed_matrix = compute_bridged_electrodes(raw)
plot_bridged_electrodes(raw.info, bridged_idx, ed_matrix)


def test_plot_ch_adjacency():
"""Test plotting of adjacency matrix."""
Expand Down
22 changes: 13 additions & 9 deletions mne/viz/topomap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2726,9 +2726,13 @@ def plot_bridged_electrodes(info, bridged_idx, ed_matrix, title=None,
%(info_not_none)s
bridged_idx : list of tuple
The indices of channels marked as bridged with each bridged
pair stored as a tuple.
pair stored as a tuple. See also
:func:`mne.preprocessing.compute_bridged_electrodes` to determine
``bridged_idx``.
ed_matrix : ndarray of float, shape (n_channels, n_channels)
The electrical distance matrix for each pair of EEG electrodes.
See also :func:`mne.preprocessing.compute_bridged_electrodes` to
determine ``ed_matrix``.
title : str
A title to add to the plot.
topomap_args : dict | None
Expand All @@ -2744,11 +2748,14 @@ def plot_bridged_electrodes(info, bridged_idx, ed_matrix, title=None,
topomap_args = dict()
else:
topomap_args = topomap_args.copy() # don't change original
picks = pick_types(info, eeg=True)
topomap_args.setdefault('image_interp', 'nearest')
topomap_args.setdefault('cmap', 'summer_r')
topomap_args.setdefault('names', info.ch_names)
topomap_args.setdefault('names', pick_info(info, picks).ch_names)

@alexrockhill alexrockhill Jun 13, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpick but you call pick_info twice, would be nice just to do once.

@mscheltienne mscheltienne Jun 13, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I thought the same, but I did not see a clean way to pick only once (at least not without changing more extensively the function). Problem is:

  • you need the 'picked' info at the beginning to set the 'names' argument (alternatively [ch for k, ch in enumerate(info.ch_names) if k in picks].. not super clean and extends a lot this line).
  • you need the 'picked' info when plotting the topomap with plot_topomap
  • you need the 'non-picked' info at the end when adding the bridged connections because bridged_idx is based on the original info with all channels.

And the gain with the list comprehension for the 'name' argument is only ≈1 ms but loses in readability.
Feel free to add further changes if you have a clever way to pick only once while managing the bridged_idx, I didn't spend much time searching for one ;)

topomap_args.setdefault('show_names', True)
topomap_args.setdefault('contours', False)
sphere = topomap_args['sphere'] if 'sphere' in topomap_args \
else _check_sphere(None)
if 'axes' not in topomap_args:
fig, ax = plt.subplots()
topomap_args['axes'] = ax
Expand All @@ -2757,9 +2764,6 @@ def plot_bridged_electrodes(info, bridged_idx, ed_matrix, title=None,
# handle colorbar here instead of in plot_topomap
colorbar = topomap_args.pop('colorbar') if \
'colorbar' in topomap_args else True
# use sphere to find positions
sphere = topomap_args['sphere'] if 'sphere' in topomap_args else None
picks = pick_types(info, eeg=True)
if ed_matrix.shape[1:] != (picks.size, picks.size):
raise RuntimeError(
f'Expected {(ed_matrix.shape[0], picks.size, picks.size)} '
Expand All @@ -2770,13 +2774,13 @@ def plot_bridged_electrodes(info, bridged_idx, ed_matrix, title=None,
for epo_idx in range(ed_matrix.shape[0]):
ed_matrix[epo_idx][tril_idx] = ed_matrix[epo_idx].T[tril_idx]
elec_dists = np.median(np.nanmin(ed_matrix, axis=1), axis=0)
pos = _find_topomap_coords(info, picks, sphere=sphere)
im, cn = plot_topomap(elec_dists, info, **topomap_args)
im, cn = plot_topomap(elec_dists, pick_info(info, picks), **topomap_args)
fig = im.figure if fig is None else fig
# add bridged connections
for idx0, idx1 in bridged_idx:
im.axes.plot([pos[idx0][0], pos[idx1][0]],
[pos[idx0][1], pos[idx1][1]], color='r')
pos = _find_topomap_coords(info, [idx0, idx1], sphere=sphere)
im.axes.plot([pos[0, 0], pos[1, 0]],
[pos[0, 1], pos[1, 1]], color='r')
if title is not None:
im.axes.set_title(title)
if colorbar:
Expand Down