Add warning if the ICA decomposition provided to ICLabel is not using an extended infomax #42
Conversation
| ica = ICA(n_components=3, method="infomax", fit_params=dict(extended=True)) | ||
| ica.fit(raw) | ||
| with pytest.warns(RuntimeWarning, match="common average reference"), pytest.warns( | ||
| RuntimeWarning, match="not filtered between 1 and 100 Hz" | ||
| ): | ||
| iclabel_label_components(raw, ica) |
There was a problem hiding this comment.
Looks like this part isn't firing a warning. Not sure why tho...
error msg from CI
with pytest.warns(RuntimeWarning, match="common average reference"), pytest.warns(
[69](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:69)
RuntimeWarning, match="not filtered between 1 and 100 Hz"
[70](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:70)
):
[71](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:71)
> iclabel_label_components(raw, ica)
[72](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:72)
E Failed: DID NOT WARN. No warnings of type (<class 'RuntimeWarning'>,) were emitted. The list of emitted warnings is: [].
[73](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:73)
[74](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:74)
data = array([[4, 9, 2, ..., 1, 2, 5],
[75](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:75)
[6, 4, 6, ..., 1, 2, 7],
[76](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:76)
[1, 9, 2, ..., 3, 9, 7]])
[77](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:77)
ica = <ICA | raw data decomposition, method: infomax (fit in 500 iterations on 5000 samples), 3 ICA components explaining 100.0 % of variance (3 PCA components available), channel types: eeg, no sources marked for exclusion>
[78](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:78)
raw = <RawArray | 3 x 5000 (10.0 s), ~128 kB, data loaded>
[79](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:79)
[80](https://github.com/mne-tools/mne-icalabel/runs/6342837668?check_suite_focus=true#step:9:80)
mne_icalabel/iclabel/tests/test_label_components.py:43: Failed
There was a problem hiding this comment.
It is firing, it's just not getting caught by pytest for some reason.. not sure why either..
There was a problem hiding this comment.
Is it working locally? / are the other warnings firing?
There was a problem hiding this comment.
The warnings are firing locally, but pytest doesn't catch them either locally or on the CI.
There was a problem hiding this comment.
@larsoner any chance you know where this might be coming from? My understanding is that mne.utils.warn should fire by default.
There was a problem hiding this comment.
I pushed a fix, but I don't know why the syntax:
with pytest.warns(RuntimeWarning, match=...), pytest.warns(RuntimeWarning, match=...):
function()
does not work.
There was a problem hiding this comment.
I'm still interesting to figure out why this syntax was failing, if anyone as any idea...
I wonder if parenthesis were necessary, e.g.
with (pytest.warns(RuntimeWarning, match=...), pytest.warns(RuntimeWarning, match=...)):
function()
There was a problem hiding this comment.
I have not looked at the error, but the syntax seems fine, but I wouldn't expect it to work since each instance will try to caputure/swallow all warnings without allowing them to propagate (I think), but only one of them will be able to do so (the other one will get no warnings because they've been "taken" by the other one)
There was a problem hiding this comment.
And you are probably right, now that I look again, it looks like it's the second pytest.warns that was complaining because the list of warnings was empty.
Although, I did try to set logger.propagate to True.
Codecov Report
@@ Coverage Diff @@
## main #42 +/- ##
==========================================
+ Coverage 97.57% 97.60% +0.02%
==========================================
Files 12 12
Lines 495 501 +6
==========================================
+ Hits 483 489 +6
Misses 12 12
Continue to review full report at Codecov.
|
|
Can you add a changelog entry in |
As pointed out by @jacobf18 in #40 (comment)
I added a warning if the ICA provided is not using an extended infomax + test for this warning and the 2 previously added warnings if the raw|epochs instance was not filtered between [1, 100] Hz or referenced to a common average.