Skip to content

Commit

Permalink
MRG: Fix side effects in ica.plot_sources (mne-tools#6595)
Browse files Browse the repository at this point in the history
* deprecate exclude param

* update tests

* remove deprecated param from tutorials

* better deprecation

* deprecate on method too

* update whats_new

* fix foolishness

* stronger warning msg; bug entry in whats_new
  • Loading branch information
drammock authored and larsoner committed Jul 25, 2019
1 parent 69d992c commit 00806ca
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 25 deletions.
3 changes: 3 additions & 0 deletions doc/whats_new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ Changelog
Bug
~~~

- Fix side-effect where :func:`mne.viz.plot_ica_sources` and :meth:`mne.preprocessing.ICA.plot_sources` changed the ``ICA.exclude`` attribute even when users didn't interact with the plot by `Daniel McCloy`_.

- Fix wrong assumptions about units in BrainVision montages and add test asserting units in "mm" or "auto", by `Stefan Appelhoff`_

- Fix scaling issue with signals in mV in EDF files read with :func:`mne.io.read_raw_edf` by `Alex Gramfort`_
Expand Down Expand Up @@ -98,6 +100,7 @@ Bug
API
~~~

- Deprecate ``exclude`` parameter in :func:`mne.viz.plot_ica_sources` and :meth:`mne.preprocessing.ICA.plot_sources`, instead always use the ``exclude`` attribute of the ICA object by `Daniel McCloy`_.

- Deprecate ``montage`` parameter in favor of the ``set_montage`` method in all EEG data readers :func:`mne.io.read_raw_cnt`, :func:`mne.io.read_raw_egi`, :func:`mne.io.read_raw_edf`, :func:`mne.io.read_raw_gdf`, :func:`mne.io.read_raw_nicolet`, :func:`mne.io.read_raw_eeglab` and :func:`mne.read_epochs_eeglab` by `Alex Gramfort`_

Expand Down
2 changes: 1 addition & 1 deletion mne/preprocessing/ica.py
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,7 @@ def plot_properties(self, inst, picks=None, axes=None, dB=True,
figsize=figsize, show=show, reject=reject)

@copy_function_doc_to_method_doc(plot_ica_sources)
def plot_sources(self, inst, picks=None, exclude=None, start=None,
def plot_sources(self, inst, picks=None, exclude='deprecated', start=None,
stop=None, title=None, show=True, block=False,
show_first_samp=False):
return plot_ica_sources(self, inst=inst, picks=picks, exclude=exclude,
Expand Down
19 changes: 11 additions & 8 deletions mne/viz/ica.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@


@fill_doc
def plot_ica_sources(ica, inst, picks=None, exclude=None, start=None,
def plot_ica_sources(ica, inst, picks=None, exclude='deprecated', start=None,
stop=None, title=None, show=True, block=False,
show_first_samp=False):
"""Plot estimated latent sources given the unmixing matrix.
Expand All @@ -47,9 +47,10 @@ def plot_ica_sources(ica, inst, picks=None, exclude=None, start=None,
inst : instance of mne.io.Raw, mne.Epochs, mne.Evoked
The object to plot the sources from.
%(picks_base)s all sources in the order as fitted.
exclude : array-like of int
The components marked for exclusion. If None (default), ICA.exclude
will be used.
exclude : 'deprecated'
The ``exclude`` parameter is deprecated and will be removed in version
0.20; specify excluded components using the ``ICA.exclude`` attribute
instead.
start : int
X-axis start index. If None, from the beginning.
stop : int
Expand Down Expand Up @@ -83,10 +84,12 @@ def plot_ica_sources(ica, inst, picks=None, exclude=None, start=None,
from ..evoked import Evoked
from ..epochs import BaseEpochs

if exclude is None:
exclude = ica.exclude
elif len(ica.exclude) > 0:
exclude = np.union1d(ica.exclude, exclude)
if exclude != 'deprecated':
warn('The "exclude" parameter is deprecated and will be removed in '
'version 0.20; specify excluded components using the ICA.exclude '
'attribute instead. Provided value of {} will be ignored; falling'
' back to ICA.exclude'.format(exclude), DeprecationWarning)
exclude = ica.exclude
picks = _picks_to_idx(ica.n_components_, picks, 'all')

if isinstance(inst, BaseRaw):
Expand Down
22 changes: 14 additions & 8 deletions mne/viz/tests/test_ica.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,21 @@ def test_plot_ica_sources():

# dtype can change int->np.int after load, test it explicitly
ica.n_components_ = np.int64(ica.n_components_)
fig = ica.plot_sources(raw, [1])
fig = ica.plot_sources(raw)
# also test mouse clicks
data_ax = fig.axes[0]
_fake_click(fig, data_ax, [-0.1, 0.9]) # click on y-label
# `exclude` parameter is deprecated
with pytest.deprecated_call():
ica.plot_sources(raw, exclude=[1])

raw.info['bads'] = ['MEG 0113']
pytest.raises(RuntimeError, ica.plot_sources, inst=raw)
with pytest.raises(RuntimeError, match="Raw doesn't match fitted data"):
ica.plot_sources(inst=raw)
ica.plot_sources(epochs)
epochs.info['bads'] = ['MEG 0113']
pytest.raises(RuntimeError, ica.plot_sources, inst=epochs)
with pytest.raises(RuntimeError, match="Epochs don't match fitted data"):
ica.plot_sources(inst=epochs)
epochs.info['bads'] = []
ica.plot_sources(epochs.average())
evoked = epochs.average()
Expand All @@ -208,13 +213,13 @@ def test_plot_ica_sources():
_fake_click(fig, ax,
[ax.get_xlim()[0], ax.get_ylim()[1]], 'data')
# plot with bad channels excluded
ica.plot_sources(evoked, exclude=[0])
ica.exclude = [0]
ica.plot_sources(evoked) # does the same thing
ica.plot_sources(evoked)
ica.labels_ = dict(eog=[0])
ica.labels_['eog/0/crazy-channel'] = [0]
ica.plot_sources(evoked) # now with labels
pytest.raises(ValueError, ica.plot_sources, 'meeow')
with pytest.raises(ValueError, match='must be of Raw or Epochs type'):
ica.plot_sources('meeow')
plt.close('all')


Expand Down Expand Up @@ -287,7 +292,8 @@ def test_plot_instance_components():
max_pca_components=3, n_pca_components=3)
with pytest.warns(RuntimeWarning, match='projection'):
ica.fit(raw, picks=picks)
fig = ica.plot_sources(raw, exclude=[0], title='Components')
ica.exclude = [0]
fig = ica.plot_sources(raw, title='Components')
for key in ['down', 'up', 'right', 'left', 'o', '-', '+', '=', 'pageup',
'pagedown', 'home', 'end', 'f11', 'b']:
fig.canvas.key_press_event(key)
Expand All @@ -299,7 +305,7 @@ def test_plot_instance_components():
fig.canvas.key_press_event('escape')
plt.close('all')
epochs = _get_epochs()
fig = ica.plot_sources(epochs, exclude=[0], title='Components')
fig = ica.plot_sources(epochs, title='Components')
for key in ['down', 'up', 'right', 'left', 'o', '-', '+', '=', 'pageup',
'pagedown', 'home', 'end', 'f11', 'b']:
fig.canvas.key_press_event(key)
Expand Down
7 changes: 5 additions & 2 deletions tutorials/preprocessing/plot_40_artifact_correction_ica.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@
# we can see that only one component is highly correlated and that this
# component got detected by our correlation analysis (red).

ica.plot_sources(eog_average, exclude=eog_inds) # look at source time course
ica.exclude = eog_inds
ica.plot_sources(eog_average) # look at source time course
ica.exclude = [] # reset exclude list for now until we're sure

###############################################################################
# We can take a look at the properties of that component, now using the
Expand Down Expand Up @@ -260,7 +262,8 @@
# Which one is the bad EOG component?
# Here we rely on our previous detection algorithm. You would need to decide
# yourself if no automatic detection was available.
reference_ica.plot_sources(eog_average, exclude=eog_inds)
reference_ica.exclude = eog_inds
reference_ica.plot_sources(eog_average)

###############################################################################
# Indeed it looks like an EOG, also in the average time course.
Expand Down
18 changes: 12 additions & 6 deletions tutorials/preprocessing/plot_45_ica_from_raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,12 @@

show_picks = np.abs(scores).argsort()[::-1][:5]

ica.plot_sources(raw, show_picks, exclude=ecg_inds, title=title % 'ecg')
ica.exclude = ecg_inds
ica.plot_sources(raw, show_picks, title=title % 'ecg')
ica.exclude = []
ica.plot_components(ecg_inds, title=title % 'ecg', colorbar=True)

ecg_inds = ecg_inds[:n_max_ecg]
ica.exclude += ecg_inds

# detect EOG by correlation

Expand All @@ -88,22 +89,27 @@

show_picks = np.abs(scores).argsort()[::-1][:5]

ica.plot_sources(raw, show_picks, exclude=eog_inds, title=title % 'eog')
ica.exclude = eog_inds
ica.plot_sources(raw, show_picks, title=title % 'eog')
ica.exclude = []
ica.plot_components(eog_inds, title=title % 'eog', colorbar=True)

eog_inds = eog_inds[:n_max_eog]
ica.exclude += eog_inds

###############################################################################
# 3) Assess component selection and unmixing quality.

# estimate average artifact
ecg_evoked = ecg_epochs.average()
ica.plot_sources(ecg_evoked, exclude=ecg_inds) # plot ECG sources + selection
ica.exclude = ecg_inds
ica.plot_sources(ecg_evoked) # plot ECG sources + selection
ica.exclude = []
ica.plot_overlay(ecg_evoked, exclude=ecg_inds) # plot ECG cleaning

eog_evoked = create_eog_epochs(raw, tmin=-.5, tmax=.5, picks=picks).average()
ica.plot_sources(eog_evoked, exclude=eog_inds) # plot EOG sources + selection
ica.exclude = eog_inds
ica.plot_sources(eog_evoked) # plot EOG sources + selection
ica.exclude = []
ica.plot_overlay(eog_evoked, exclude=eog_inds) # plot EOG cleaning

# check the amplitudes do not change
Expand Down

0 comments on commit 00806ca

Please sign in to comment.