From 44c8ffa243555643c5ad0b35b52399558d42c888 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Sun, 12 Jul 2020 22:44:23 +0200 Subject: [PATCH 01/14] When picking, remove inapplicable projectors --- mne/channels/channels.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/mne/channels/channels.py b/mne/channels/channels.py index 552d3cce9c9..b4f6f3e04f9 100644 --- a/mne/channels/channels.py +++ b/mne/channels/channels.py @@ -722,7 +722,9 @@ def pick_types(self, meg=None, eeg=False, stim=False, eog=False, ias=ias, syst=syst, seeg=seeg, dipole=dipole, gof=gof, bio=bio, ecog=ecog, fnirs=fnirs, include=include, exclude=exclude, selection=selection) - return self._pick_drop_channels(idx) + self._pick_drop_channels(idx) + self._pick_projs() + return self def pick_channels(self, ch_names, ordered=False): """Pick some channels. @@ -756,8 +758,10 @@ def pick_channels(self, ch_names, ordered=False): .. versionadded:: 0.9.0 """ - return self._pick_drop_channels( - pick_channels(self.info['ch_names'], ch_names, ordered=ordered)) + self._pick_drop_channels(pick_channels(self.info['ch_names'], ch_names, + ordered=ordered)) + self._pick_projs() + return self @fill_doc def pick(self, picks, exclude=()): @@ -777,7 +781,9 @@ def pick(self, picks, exclude=()): """ picks = _picks_to_idx(self.info, picks, 'all', exclude, allow_empty=False) - return self._pick_drop_channels(picks) + self._pick_drop_channels(picks) + self._pick_projs() + return self def reorder_channels(self, ch_names): """Reorder channels. @@ -890,6 +896,20 @@ def _pick_drop_channels(self, idx): assert isinstance(self, BaseRaw) and not self.preload return self + def _pick_projs(self): + """Only keep projectors whose channels are still in the data.""" + drop_idx = [] + for idx, proj in enumerate(self.info['projs']): + if not set(self.info['ch_names']) & set(proj['data']['col_names']): + drop_idx.append(idx) + + for idx in drop_idx: + logger.warning(f"Removing projector {self.info['projs'][idx]}") + if drop_idx: + self.del_proj(drop_idx) + + return self + def add_channels(self, add_list, force_update_info=False): """Append new channels to the instance. From c4d5351ad622a99dc83f23bffa546126a8dd306a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 19:31:10 +0200 Subject: [PATCH 02/14] Add test --- mne/io/fiff/tests/test_raw_fiff.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/mne/io/fiff/tests/test_raw_fiff.py b/mne/io/fiff/tests/test_raw_fiff.py index 0ff941729bd..0afe8c9957f 100644 --- a/mne/io/fiff/tests/test_raw_fiff.py +++ b/mne/io/fiff/tests/test_raw_fiff.py @@ -728,10 +728,25 @@ def test_proj(tmpdir): assert_allclose(data_proj_1, data_proj_2) assert_allclose(data_proj_2, np.dot(raw._projector, data_proj_2)) + # Test that picking removes projectors ... + raw = read_raw_fif(fif_fname).apply_proj() + n_projs = len(raw.info['projs']) + raw.pick_types(meg=False, eeg=True) + assert len(raw.info['projs']) == n_projs - 3 + + # ... but only if it doesn't apply to any channels in the dataset anymore. + raw = read_raw_fif(fif_fname).apply_proj() + n_projs = len(raw.info['projs']) + raw.pick_types(meg='mag', eeg=True) + assert len(raw.info['projs']) == n_projs + + # I/O roundtrip of an MEG projector with a Raw that only contains MEG + # data. out_fname = tmpdir.join('test_raw.fif') raw = read_raw_fif(test_fif_fname, preload=True).crop(0, 0.002) + proj = raw.info['projs'][-1] raw.pick_types(meg=False, eeg=True) - raw.info['projs'] = [raw.info['projs'][-1]] + raw.info['projs'] = [proj] # Restore, because picking removed it! raw._data.fill(0) raw._data[-1] = 1. raw.save(out_fname) @@ -739,18 +754,6 @@ def test_proj(tmpdir): raw.apply_proj() assert_allclose(raw[:, :][0][:1], raw[0, :][0]) - # Read file again, apply proj, pick all channels one proj did NOT apply to; - # then try to delete this proj, which now exclusively refers to channels - # which are not present in the data anymore. - raw = read_raw_fif(fif_fname).apply_proj() - del_proj_idx = 0 - picks = list(set(raw.ch_names) - - set(raw.info['projs'][del_proj_idx]['data']['col_names'])) - raw.pick(picks) - n_projs = len(raw.info['projs']) - raw.del_proj(del_proj_idx) - assert len(raw.info['projs']) == n_projs - 1 - @testing.requires_testing_data @pytest.mark.parametrize('preload', [False, True, 'memmap.dat']) From ceedd171bf50d34e6dd24d70f7958e9555e3093d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 19:31:18 +0200 Subject: [PATCH 03/14] Change docstring --- mne/io/proj.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mne/io/proj.py b/mne/io/proj.py index eadf918fc5e..ae549408dd9 100644 --- a/mne/io/proj.py +++ b/mne/io/proj.py @@ -222,8 +222,7 @@ def del_proj(self, idx='all'): """Remove SSP projection vector. .. note:: The projection vector can only be removed if it is inactive - (has not been applied to the data), unless the channels it - was applied to no longer exist in the data. + (has not been applied to the data). Parameters ---------- From 809793209c1f4ff4bb854a231d349bbe6e3e43c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 19:32:31 +0200 Subject: [PATCH 04/14] Typo --- mne/io/fiff/tests/test_raw_fiff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/io/fiff/tests/test_raw_fiff.py b/mne/io/fiff/tests/test_raw_fiff.py index 0afe8c9957f..2c5667f4b4a 100644 --- a/mne/io/fiff/tests/test_raw_fiff.py +++ b/mne/io/fiff/tests/test_raw_fiff.py @@ -740,7 +740,7 @@ def test_proj(tmpdir): raw.pick_types(meg='mag', eeg=True) assert len(raw.info['projs']) == n_projs - # I/O roundtrip of an MEG projector with a Raw that only contains MEG + # I/O roundtrip of an MEG projector with a Raw that only contains EEG # data. out_fname = tmpdir.join('test_raw.fif') raw = read_raw_fif(test_fif_fname, preload=True).crop(0, 0.002) From bdf900f9718e678e5ed4d15a72af654d5e3c93d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 19:52:07 +0200 Subject: [PATCH 05/14] Capture warnings in test --- mne/channels/channels.py | 2 +- mne/io/fiff/tests/test_raw_fiff.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/mne/channels/channels.py b/mne/channels/channels.py index b4f6f3e04f9..39e1d05b9c0 100644 --- a/mne/channels/channels.py +++ b/mne/channels/channels.py @@ -904,7 +904,7 @@ def _pick_projs(self): drop_idx.append(idx) for idx in drop_idx: - logger.warning(f"Removing projector {self.info['projs'][idx]}") + warn(f"Removing projector {self.info['projs'][idx]}") if drop_idx: self.del_proj(drop_idx) diff --git a/mne/io/fiff/tests/test_raw_fiff.py b/mne/io/fiff/tests/test_raw_fiff.py index 2c5667f4b4a..039f19fd27e 100644 --- a/mne/io/fiff/tests/test_raw_fiff.py +++ b/mne/io/fiff/tests/test_raw_fiff.py @@ -731,7 +731,8 @@ def test_proj(tmpdir): # Test that picking removes projectors ... raw = read_raw_fif(fif_fname).apply_proj() n_projs = len(raw.info['projs']) - raw.pick_types(meg=False, eeg=True) + with pytest.warns(RuntimeWarning, match='Removing projector'): + raw.pick_types(meg=False, eeg=True) assert len(raw.info['projs']) == n_projs - 3 # ... but only if it doesn't apply to any channels in the dataset anymore. @@ -745,7 +746,8 @@ def test_proj(tmpdir): out_fname = tmpdir.join('test_raw.fif') raw = read_raw_fif(test_fif_fname, preload=True).crop(0, 0.002) proj = raw.info['projs'][-1] - raw.pick_types(meg=False, eeg=True) + with pytest.warns(RuntimeWarning, match='Removing projector'): + raw.pick_types(meg=False, eeg=True) raw.info['projs'] = [proj] # Restore, because picking removed it! raw._data.fill(0) raw._data[-1] = 1. From 317683032870ce08035828ef745ed0c4bfa3b5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 20:14:26 +0200 Subject: [PATCH 06/14] Emit info, not warning --- mne/channels/channels.py | 2 +- mne/io/fiff/tests/test_raw_fiff.py | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/mne/channels/channels.py b/mne/channels/channels.py index 39e1d05b9c0..3c30e8a39a8 100644 --- a/mne/channels/channels.py +++ b/mne/channels/channels.py @@ -904,7 +904,7 @@ def _pick_projs(self): drop_idx.append(idx) for idx in drop_idx: - warn(f"Removing projector {self.info['projs'][idx]}") + logger.info(f"Removing projector {self.info['projs'][idx]}") if drop_idx: self.del_proj(drop_idx) diff --git a/mne/io/fiff/tests/test_raw_fiff.py b/mne/io/fiff/tests/test_raw_fiff.py index 039f19fd27e..2c5667f4b4a 100644 --- a/mne/io/fiff/tests/test_raw_fiff.py +++ b/mne/io/fiff/tests/test_raw_fiff.py @@ -731,8 +731,7 @@ def test_proj(tmpdir): # Test that picking removes projectors ... raw = read_raw_fif(fif_fname).apply_proj() n_projs = len(raw.info['projs']) - with pytest.warns(RuntimeWarning, match='Removing projector'): - raw.pick_types(meg=False, eeg=True) + raw.pick_types(meg=False, eeg=True) assert len(raw.info['projs']) == n_projs - 3 # ... but only if it doesn't apply to any channels in the dataset anymore. @@ -746,8 +745,7 @@ def test_proj(tmpdir): out_fname = tmpdir.join('test_raw.fif') raw = read_raw_fif(test_fif_fname, preload=True).crop(0, 0.002) proj = raw.info['projs'][-1] - with pytest.warns(RuntimeWarning, match='Removing projector'): - raw.pick_types(meg=False, eeg=True) + raw.pick_types(meg=False, eeg=True) raw.info['projs'] = [proj] # Restore, because picking removed it! raw._data.fill(0) raw._data[-1] = 1. From 74e4ec8520c7272df9428b1ef444e54dab2bc028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 20:47:19 +0200 Subject: [PATCH 07/14] Update changelog --- doc/changes/latest.inc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/changes/latest.inc b/doc/changes/latest.inc index 2be7c5824f3..4e82ff4c72f 100644 --- a/doc/changes/latest.inc +++ b/doc/changes/latest.inc @@ -127,6 +127,8 @@ Changelog - Deletion of applied (active) projectors via `~mne.io.Raw.del_proj`, `~mne.Epochs.del_proj`, and `~mne.Evoked.del_proj` is now possible if the channels the to-be-removed projector applies to are not present in the data anymore by `Richard Höchenberger`_ +- When picking a subset of channels from `~mne.io.Raw`, `~mne.Epochs`, or `~mne.Evoked`, projectors that can only be applied to the removed channels will now be dropped automatically by `Richard Höchenberger`_ + Bug ~~~ From 86258c80a6ff7b8caef0709a969d36192b0cb7e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 23:34:48 +0200 Subject: [PATCH 08/14] Handle drop_channels() too --- doc/changes/latest.inc | 2 +- mne/channels/channels.py | 4 +++- mne/io/fiff/tests/test_raw_fiff.py | 7 +++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/doc/changes/latest.inc b/doc/changes/latest.inc index 4e82ff4c72f..d89008de9db 100644 --- a/doc/changes/latest.inc +++ b/doc/changes/latest.inc @@ -127,7 +127,7 @@ Changelog - Deletion of applied (active) projectors via `~mne.io.Raw.del_proj`, `~mne.Epochs.del_proj`, and `~mne.Evoked.del_proj` is now possible if the channels the to-be-removed projector applies to are not present in the data anymore by `Richard Höchenberger`_ -- When picking a subset of channels from `~mne.io.Raw`, `~mne.Epochs`, or `~mne.Evoked`, projectors that can only be applied to the removed channels will now be dropped automatically by `Richard Höchenberger`_ +- When picking a subset of channels, or when dropping channels from `~mne.io.Raw`, `~mne.Epochs`, or `~mne.Evoked`, projectors that can only be applied to the removed channels will now be dropped automatically by `Richard Höchenberger`_ Bug ~~~ diff --git a/mne/channels/channels.py b/mne/channels/channels.py index 3c30e8a39a8..2e6a733a4d5 100644 --- a/mne/channels/channels.py +++ b/mne/channels/channels.py @@ -864,7 +864,9 @@ def drop_channels(self, ch_names): bad_idx = [self.ch_names.index(ch) for ch in ch_names if ch in self.ch_names] idx = np.setdiff1d(np.arange(len(self.ch_names)), bad_idx) - return self._pick_drop_channels(idx) + self._pick_drop_channels(idx) + self._pick_projs() + return self def _pick_drop_channels(self, idx): # avoid circular imports diff --git a/mne/io/fiff/tests/test_raw_fiff.py b/mne/io/fiff/tests/test_raw_fiff.py index 2c5667f4b4a..d0cd207b4ea 100644 --- a/mne/io/fiff/tests/test_raw_fiff.py +++ b/mne/io/fiff/tests/test_raw_fiff.py @@ -1478,6 +1478,13 @@ def test_drop_channels_mixin(): assert len(ch_names) == len(raw._cals) assert len(ch_names) == raw._data.shape[0] + # Test that picking all channels a projector applies to will lead to the + # removal of said projector. + raw = read_raw_fif(fif_fname).apply_proj() + n_projs = len(raw.info['projs']) + raw.drop_channels(raw.info['projs'][-1]['data']['col_names']) # EEG proj + assert len(raw.info['projs']) == n_projs - 1 + @testing.requires_testing_data @pytest.mark.parametrize('preload', (True, False)) From f43c03c85fa85735be855c8ed645372dcda0ed86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Mon, 13 Jul 2020 23:39:40 +0200 Subject: [PATCH 09/14] Typo --- mne/io/fiff/tests/test_raw_fiff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/io/fiff/tests/test_raw_fiff.py b/mne/io/fiff/tests/test_raw_fiff.py index d0cd207b4ea..9a77f2cc91c 100644 --- a/mne/io/fiff/tests/test_raw_fiff.py +++ b/mne/io/fiff/tests/test_raw_fiff.py @@ -1478,7 +1478,7 @@ def test_drop_channels_mixin(): assert len(ch_names) == len(raw._cals) assert len(ch_names) == raw._data.shape[0] - # Test that picking all channels a projector applies to will lead to the + # Test that dropping all channels a projector applies to will lead to the # removal of said projector. raw = read_raw_fif(fif_fname).apply_proj() n_projs = len(raw.info['projs']) From bed0dcdab20d17eac63fec9228a28367756ab9e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Tue, 14 Jul 2020 00:15:52 +0200 Subject: [PATCH 10/14] More DRY --- mne/channels/channels.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/mne/channels/channels.py b/mne/channels/channels.py index 2e6a733a4d5..252ccf6ec02 100644 --- a/mne/channels/channels.py +++ b/mne/channels/channels.py @@ -722,9 +722,7 @@ def pick_types(self, meg=None, eeg=False, stim=False, eog=False, ias=ias, syst=syst, seeg=seeg, dipole=dipole, gof=gof, bio=bio, ecog=ecog, fnirs=fnirs, include=include, exclude=exclude, selection=selection) - self._pick_drop_channels(idx) - self._pick_projs() - return self + return self._pick_drop_channels(idx) def pick_channels(self, ch_names, ordered=False): """Pick some channels. @@ -758,10 +756,8 @@ def pick_channels(self, ch_names, ordered=False): .. versionadded:: 0.9.0 """ - self._pick_drop_channels(pick_channels(self.info['ch_names'], ch_names, - ordered=ordered)) - self._pick_projs() - return self + picks = pick_channels(self.info['ch_names'], ch_names, ordered=ordered) + return self._pick_drop_channels(picks) @fill_doc def pick(self, picks, exclude=()): @@ -781,9 +777,7 @@ def pick(self, picks, exclude=()): """ picks = _picks_to_idx(self.info, picks, 'all', exclude, allow_empty=False) - self._pick_drop_channels(picks) - self._pick_projs() - return self + return self._pick_drop_channels(picks) def reorder_channels(self, ch_names): """Reorder channels. @@ -864,9 +858,7 @@ def drop_channels(self, ch_names): bad_idx = [self.ch_names.index(ch) for ch in ch_names if ch in self.ch_names] idx = np.setdiff1d(np.arange(len(self.ch_names)), bad_idx) - self._pick_drop_channels(idx) - self._pick_projs() - return self + return self._pick_drop_channels(idx) def _pick_drop_channels(self, idx): # avoid circular imports @@ -896,6 +888,8 @@ def _pick_drop_channels(self, idx): self._data = self._data.take(idx, axis=axis) else: assert isinstance(self, BaseRaw) and not self.preload + + self._pick_projs() return self def _pick_projs(self): From 2afc2d4806a6ed20c30f874f2d148673497d3fc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Tue, 14 Jul 2020 10:09:15 +0200 Subject: [PATCH 11/14] Check if del_proj exists --- mne/channels/channels.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mne/channels/channels.py b/mne/channels/channels.py index 252ccf6ec02..50dae191949 100644 --- a/mne/channels/channels.py +++ b/mne/channels/channels.py @@ -901,7 +901,8 @@ def _pick_projs(self): for idx in drop_idx: logger.info(f"Removing projector {self.info['projs'][idx]}") - if drop_idx: + + if drop_idx and hasattr(self, 'del_proj'): self.del_proj(drop_idx) return self From c708c71c6f7b046928f75d5c0b98cae63b80a9d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Tue, 14 Jul 2020 10:36:42 +0200 Subject: [PATCH 12/14] Better docstring --- mne/channels/channels.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/channels/channels.py b/mne/channels/channels.py index 50dae191949..c5ed2481531 100644 --- a/mne/channels/channels.py +++ b/mne/channels/channels.py @@ -893,7 +893,7 @@ def _pick_drop_channels(self, idx): return self def _pick_projs(self): - """Only keep projectors whose channels are still in the data.""" + """Keep only projectors which apply to at least 1 data channel.""" drop_idx = [] for idx, proj in enumerate(self.info['projs']): if not set(self.info['ch_names']) & set(proj['data']['col_names']): From 719c41c12f9aa17c288d7901dec1869c054f6598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Tue, 14 Jul 2020 10:38:00 +0200 Subject: [PATCH 13/14] Drop line from changelog This case shouldn't occur anymore if using the public API, as picking channels now already trims projectors. --- doc/changes/latest.inc | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/changes/latest.inc b/doc/changes/latest.inc index d89008de9db..c8307aece12 100644 --- a/doc/changes/latest.inc +++ b/doc/changes/latest.inc @@ -125,8 +125,6 @@ Changelog - BrainVision data format files are now parsed for EEG impedance values in :func:`mne.io.read_raw_brainvision` and provided as a ``.impedances`` attribute of ``raw`` by `Stefan Appelhoff`_ and `Jan Sedivy`_ -- Deletion of applied (active) projectors via `~mne.io.Raw.del_proj`, `~mne.Epochs.del_proj`, and `~mne.Evoked.del_proj` is now possible if the channels the to-be-removed projector applies to are not present in the data anymore by `Richard Höchenberger`_ - - When picking a subset of channels, or when dropping channels from `~mne.io.Raw`, `~mne.Epochs`, or `~mne.Evoked`, projectors that can only be applied to the removed channels will now be dropped automatically by `Richard Höchenberger`_ Bug From 5ef3fc6dca44f09c78aa33960bf657c209a1fd21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Tue, 14 Jul 2020 12:46:51 +0200 Subject: [PATCH 14/14] Fix LCMV test --- mne/beamformer/tests/test_lcmv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/beamformer/tests/test_lcmv.py b/mne/beamformer/tests/test_lcmv.py index bbd5c48c16f..d54ffb9f65e 100644 --- a/mne/beamformer/tests/test_lcmv.py +++ b/mne/beamformer/tests/test_lcmv.py @@ -293,7 +293,7 @@ def test_make_lcmv(tmpdir, reg, proj): # __repr__ assert len(evoked.ch_names) == 22 - assert len(evoked.info['projs']) == (4 if proj else 0) + assert len(evoked.info['projs']) == (3 if proj else 0) assert len(evoked.info['bads']) == 2 rank = 17 if proj else 20 assert 'LCMV' in repr(filters)