From f091bd1256915502cdc3c39cdeae8f6f2e486b6b Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Sun, 28 Mar 2021 02:07:19 +0530 Subject: [PATCH 01/18] Fix issue where set ylim parameter gets swapped across channel types --- mne/viz/topo.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index f4931e16d91..916df08280e 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -724,7 +724,10 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, nirs_types = {'hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od'} is_nirs = len(set.intersection(types_used, nirs_types)) > 0 if is_meg: - types_used = list(types_used)[::-1] # -> restore kwarg order + types_used = list(types_used) + # Fix issue where ylimits get swapped + if types_used[0] == 'grad': + types_used = list(types_used)[::-1] picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[]) for kk in types_used] elif is_nirs: From acb945788b949d3f6a5b6da87c8929c1150130bd Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Mon, 29 Mar 2021 17:43:18 +0530 Subject: [PATCH 02/18] Fix minor introduced in PR #9162 --- mne/viz/topo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 916df08280e..299fc8584aa 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -231,7 +231,7 @@ def _plot_topo(info, times, show_func, click_func=None, layout=None, unified=unified, img=img, axes=axes) # Temporarily converting the ylim to a list to avoid zip object exhaustion - if ylim is not None: + if layout.kind == 'Vectorview-all' and ylim is not None: ylim_list = [list(t) for t in zip(*ylim)] else: ylim_list = ylim From 2eca04b172c00b12a05ad1ddbddefc4cc6e420f2 Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Mon, 29 Mar 2021 17:44:04 +0530 Subject: [PATCH 03/18] Remove set operation to reduce randomness --- mne/viz/topo.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 299fc8584aa..60ce06a863c 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -713,7 +713,8 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, if not merge_channels: # XXX. at the moment we are committed to 1- / 2-sensor-types layouts - chs_in_layout = set(layout.names) & set(ch_names) + chs_in_layout = [ch_name for ch_name in ch_names + if ch_name in layout.names] types_used = {channel_type(info, ch_names.index(ch)) for ch in chs_in_layout} # remove possible reference meg channels From 1e497c7524f2ba6cec266459f05d7e6c2bc1ff79 Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Mon, 29 Mar 2021 17:57:15 +0530 Subject: [PATCH 04/18] Fix style issue --- mne/viz/topo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 60ce06a863c..239fb24d81f 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -714,7 +714,7 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, if not merge_channels: # XXX. at the moment we are committed to 1- / 2-sensor-types layouts chs_in_layout = [ch_name for ch_name in ch_names - if ch_name in layout.names] + if ch_name in layout.names] types_used = {channel_type(info, ch_names.index(ch)) for ch in chs_in_layout} # remove possible reference meg channels From 903e00f10c69d58cb2951cf9aea3191a8b9bcc3f Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Wed, 31 Mar 2021 04:30:47 +0530 Subject: [PATCH 05/18] Removing the zip conversion process --- mne/viz/topo.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 239fb24d81f..95ac7bd82cd 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -230,16 +230,9 @@ def _plot_topo(info, times, show_func, click_func=None, layout=None, fig_facecolor=fig_facecolor, unified=unified, img=img, axes=axes) - # Temporarily converting the ylim to a list to avoid zip object exhaustion - if layout.kind == 'Vectorview-all' and ylim is not None: - ylim_list = [list(t) for t in zip(*ylim)] - else: - ylim_list = ylim - for ax, ch_idx in my_topo_plot: - if layout.kind == 'Vectorview-all' and ylim_list is not None: + if layout.kind == 'Vectorview-all' and ylim is not None: this_type = {'mag': 0, 'grad': 1}[channel_type(info, ch_idx)] - ylim = zip(*ylim_list) ylim_ = [v[this_type] if _check_vlim(v) else v for v in ylim] else: ylim_ = ylim @@ -772,7 +765,9 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, if len(ylim_) == 1: ylim_ = ylim_[0] else: - ylim_ = zip(*[np.array(yl) for yl in ylim_]) + ylim_ = [np.array(yl) for yl in ylim_] + # Transposing to avoid Zipping confusion + ylim_ = list(map(list, zip(*ylim_))) else: raise TypeError('ylim must be None or a dict. Got %s.' % type(ylim)) From 9e4ee2a83ddb60cf04fe4b019beb3dc500f0692d Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Wed, 31 Mar 2021 04:38:01 +0530 Subject: [PATCH 06/18] Additional check to prevent changed to fnirs data --- mne/viz/topo.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 95ac7bd82cd..39d5ec31944 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -767,7 +767,8 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, else: ylim_ = [np.array(yl) for yl in ylim_] # Transposing to avoid Zipping confusion - ylim_ = list(map(list, zip(*ylim_))) + if is_meg: + ylim_ = list(map(list, zip(*ylim_))) else: raise TypeError('ylim must be None or a dict. Got %s.' % type(ylim)) From e981f71622f39b34f6edd0c2d917bb9ff9072acd Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Wed, 31 Mar 2021 18:45:32 +0530 Subject: [PATCH 07/18] Add Transpose to nirs data as well --- mne/viz/topo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 39d5ec31944..5498c0c7718 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -767,7 +767,7 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, else: ylim_ = [np.array(yl) for yl in ylim_] # Transposing to avoid Zipping confusion - if is_meg: + if is_meg or is_nirs: ylim_ = list(map(list, zip(*ylim_))) else: raise TypeError('ylim must be None or a dict. Got %s.' % type(ylim)) From 34a0e20261e9b1d0440d517956f8f3caec056cce Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Wed, 31 Mar 2021 23:57:05 +0530 Subject: [PATCH 08/18] Restore order for fnirs ylims if flipped --- mne/viz/topo.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 5498c0c7718..0a9116878f2 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -725,7 +725,10 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[]) for kk in types_used] elif is_nirs: - types_used = list(types_used)[::-1] # -> restore kwarg order + types_used = list(types_used) + # Fix issue where ylimits get swapped + if types_used[0] == 'hbr': + types_used = list(types_used)[::-1] picks = [pick_types(info, fnirs=kk, ref_meg=False, exclude=[]) for kk in types_used] else: From dfd2979001c55ce0eb72383c8ff482a57480924c Mon Sep 17 00:00:00 2001 From: Ram Date: Thu, 8 Apr 2021 04:05:45 +0530 Subject: [PATCH 09/18] Added an XXX marker to work on later Co-authored-by: Mainak Jas --- mne/viz/topo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 0a9116878f2..9b424e61b73 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -719,7 +719,7 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, is_nirs = len(set.intersection(types_used, nirs_types)) > 0 if is_meg: types_used = list(types_used) - # Fix issue where ylimits get swapped + # XXX: Fix issue where ylimits get swapped if types_used[0] == 'grad': types_used = list(types_used)[::-1] picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[]) From 5ec7e28398d8f11f3fe0bdb5742c238f653aa34c Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Thu, 8 Apr 2021 23:39:09 +0530 Subject: [PATCH 10/18] Coverting set operations to remove randomness --- mne/viz/topo.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 0a9116878f2..0b707442d36 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -708,24 +708,25 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, # XXX. at the moment we are committed to 1- / 2-sensor-types layouts chs_in_layout = [ch_name for ch_name in ch_names if ch_name in layout.names] - types_used = {channel_type(info, ch_names.index(ch)) - for ch in chs_in_layout} + # Using List comprehension to order elements and prevent randomness + types_used = [channel_type(info, ch_names.index(ch)) + for ch in chs_in_layout] + # Using dict conversion to remove duplicates + types_used = list(dict.fromkeys(types_used)) # remove possible reference meg channels - types_used = set.difference(types_used, set('ref_meg')) + types_used = list(filter(lambda x: x != 'ref_meg', types_used)) # one check for all vendors - meg_types = {'mag', 'grad'} - is_meg = len(set.intersection(types_used, meg_types)) > 0 - nirs_types = {'hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od'} - is_nirs = len(set.intersection(types_used, nirs_types)) > 0 + meg_types = ['mag', 'grad'] + is_meg = len(list(filter(lambda x: x in meg_types,types_used))) > 0 + nirs_types = ['hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od'] + is_nirs = len(list(filter(lambda x: x in nirs_types,types_used))) > 0 if is_meg: - types_used = list(types_used) # Fix issue where ylimits get swapped if types_used[0] == 'grad': types_used = list(types_used)[::-1] picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[]) for kk in types_used] elif is_nirs: - types_used = list(types_used) # Fix issue where ylimits get swapped if types_used[0] == 'hbr': types_used = list(types_used)[::-1] From ccdca461cccfd6a446d125db8cf874ca0bdd8a3a Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Fri, 9 Apr 2021 00:00:44 +0530 Subject: [PATCH 11/18] Fix style issue --- mne/viz/topo.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 0b707442d36..70de8b520c7 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -717,9 +717,9 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, types_used = list(filter(lambda x: x != 'ref_meg', types_used)) # one check for all vendors meg_types = ['mag', 'grad'] - is_meg = len(list(filter(lambda x: x in meg_types,types_used))) > 0 + is_meg = len(list(filter(lambda x: x in meg_types, types_used))) > 0 nirs_types = ['hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od'] - is_nirs = len(list(filter(lambda x: x in nirs_types,types_used))) > 0 + is_nirs = len(list(filter(lambda x: x in nirs_types, types_used))) > 0 if is_meg: # Fix issue where ylimits get swapped if types_used[0] == 'grad': From 92f2517b492b9c3f2b9d701d988d7f009d5742b3 Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Fri, 9 Apr 2021 00:29:33 +0530 Subject: [PATCH 12/18] Removing additional checks --- mne/viz/topo.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 70de8b520c7..0b6a7504858 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -722,14 +722,12 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, is_nirs = len(list(filter(lambda x: x in nirs_types, types_used))) > 0 if is_meg: # Fix issue where ylimits get swapped - if types_used[0] == 'grad': - types_used = list(types_used)[::-1] + types_used = list(types_used)[::-1] picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[]) for kk in types_used] elif is_nirs: # Fix issue where ylimits get swapped - if types_used[0] == 'hbr': - types_used = list(types_used)[::-1] + types_used = list(types_used)[::-1] picks = [pick_types(info, fnirs=kk, ref_meg=False, exclude=[]) for kk in types_used] else: From fed329649ea1d6987b07528519175d423c32f8f1 Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Fri, 9 Apr 2021 00:47:31 +0530 Subject: [PATCH 13/18] Converting lambda filters to list comp --- mne/viz/topo.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 0b6a7504858..3251cd99083 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -714,12 +714,12 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, # Using dict conversion to remove duplicates types_used = list(dict.fromkeys(types_used)) # remove possible reference meg channels - types_used = list(filter(lambda x: x != 'ref_meg', types_used)) + types_used = [types_used for types_used in types_used + if types_used != 'ref_meg'] # one check for all vendors - meg_types = ['mag', 'grad'] - is_meg = len(list(filter(lambda x: x in meg_types, types_used))) > 0 - nirs_types = ['hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od'] - is_nirs = len(list(filter(lambda x: x in nirs_types, types_used))) > 0 + is_meg = len([x for x in types_used if x in ['mag', 'grad']]) > 0 + is_nirs = len([x for x in types_used if x in + ['hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od']]) > 0 if is_meg: # Fix issue where ylimits get swapped types_used = list(types_used)[::-1] From 31df8c9460c2cac2fab98bea9dd6becdaf57f78d Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Fri, 9 Apr 2021 00:57:39 +0530 Subject: [PATCH 14/18] Update changelog with details --- doc/changes/latest.inc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/changes/latest.inc b/doc/changes/latest.inc index 1a54e4f00b0..3fdbbbaba0e 100644 --- a/doc/changes/latest.inc +++ b/doc/changes/latest.inc @@ -173,6 +173,8 @@ Enhancements Bugs ~~~~ +- Fix bug with :func:`plot_evoked_topo` where set ylim parameters gets swapped across channel types. (:gh:`9207` by |Ram Pari|_) + - Fix bug with :func:`mne.io.read_raw_edf` where µV was not correctly recognized (:gh:`9187` **by new contributor** |Sumalyo Datta|_) - Fix bug with :func:`mne.viz.plot_compare_evokeds` did not check type of combine. (:gh:`9151` **by new contributor** |Matteo Anelli|_) From 4469ef65ba58d4c9b998de6218b0d65d35f0c7ce Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Fri, 9 Apr 2021 01:00:29 +0530 Subject: [PATCH 15/18] Fix typo in changelog --- doc/changes/latest.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/changes/latest.inc b/doc/changes/latest.inc index 3fdbbbaba0e..ad5f0e21e33 100644 --- a/doc/changes/latest.inc +++ b/doc/changes/latest.inc @@ -173,7 +173,7 @@ Enhancements Bugs ~~~~ -- Fix bug with :func:`plot_evoked_topo` where set ylim parameters gets swapped across channel types. (:gh:`9207` by |Ram Pari|_) +- Fix bug with :func:`mne.viz.plot_evoked_topo` where set ylim parameters gets swapped across channel types. (:gh:`9207` by |Ram Pari|_) - Fix bug with :func:`mne.io.read_raw_edf` where µV was not correctly recognized (:gh:`9187` **by new contributor** |Sumalyo Datta|_) From 744da2bd53cabd84b4f1fde9246271b15428f3c3 Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Fri, 9 Apr 2021 02:04:14 +0530 Subject: [PATCH 16/18] Changes to comments --- mne/viz/topo.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 3251cd99083..e4838f82cd3 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -708,7 +708,6 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, # XXX. at the moment we are committed to 1- / 2-sensor-types layouts chs_in_layout = [ch_name for ch_name in ch_names if ch_name in layout.names] - # Using List comprehension to order elements and prevent randomness types_used = [channel_type(info, ch_names.index(ch)) for ch in chs_in_layout] # Using dict conversion to remove duplicates @@ -721,13 +720,11 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, is_nirs = len([x for x in types_used if x in ['hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od']]) > 0 if is_meg: - # Fix issue where ylimits get swapped - types_used = list(types_used)[::-1] + types_used = list(types_used)[::-1] # -> restore kwarg order picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[]) for kk in types_used] elif is_nirs: - # Fix issue where ylimits get swapped - types_used = list(types_used)[::-1] + types_used = list(types_used)[::-1] # -> restore kwarg order picks = [pick_types(info, fnirs=kk, ref_meg=False, exclude=[]) for kk in types_used] else: From 7c7d1586dc6df8882d45a1d9ad6c315794f16a24 Mon Sep 17 00:00:00 2001 From: Ram Pari Date: Fri, 9 Apr 2021 02:07:58 +0530 Subject: [PATCH 17/18] Fixes style issue --- mne/viz/topo.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index e4838f82cd3..8a8a6f6319b 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -720,11 +720,11 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, is_nirs = len([x for x in types_used if x in ['hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od']]) > 0 if is_meg: - types_used = list(types_used)[::-1] # -> restore kwarg order + types_used = list(types_used)[::-1] # -> restore kwarg order picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[]) for kk in types_used] elif is_nirs: - types_used = list(types_used)[::-1] # -> restore kwarg order + types_used = list(types_used)[::-1] # -> restore kwarg order picks = [pick_types(info, fnirs=kk, ref_meg=False, exclude=[]) for kk in types_used] else: From 681e4b07c1bcadeb9923e47a5ab4af08ffb54639 Mon Sep 17 00:00:00 2001 From: Ram Date: Sat, 10 Apr 2021 15:07:11 +0530 Subject: [PATCH 18/18] Update mne/viz/topo.py Co-authored-by: Alexandre Gramfort --- mne/viz/topo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/topo.py b/mne/viz/topo.py index 8a8a6f6319b..5ee93709abd 100644 --- a/mne/viz/topo.py +++ b/mne/viz/topo.py @@ -718,7 +718,7 @@ def _plot_evoked_topo(evoked, layout=None, layout_scale=0.945, color=None, # one check for all vendors is_meg = len([x for x in types_used if x in ['mag', 'grad']]) > 0 is_nirs = len([x for x in types_used if x in - ['hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od']]) > 0 + ('hbo', 'hbr', 'fnirs_cw_amplitude', 'fnirs_od')]) > 0 if is_meg: types_used = list(types_used)[::-1] # -> restore kwarg order picks = [pick_types(info, meg=kk, ref_meg=False, exclude=[])