Fig bug with ica.plot_components when axes is provided#11654
Fig bug with ica.plot_components when axes is provided#11654larsoner merged 11 commits intomne-tools:mainfrom
Conversation
larsoner
left a comment
There was a problem hiding this comment.
Other than one minor gripe about potential testing time LGTM, thanks @mscheltienne !
mne/viz/tests/test_ica.py
Outdated
| ica = ICA(n_components=None) | ||
| ica_picks = _get_picks(raw) # 9 channels | ||
| ica.fit(raw, picks=ica_picks) |
There was a problem hiding this comment.
Any way you can get away with not doing another .fit? All these ICA fits add up in testing time
There was a problem hiding this comment.
I tried to, but the other one above only fits 2 components which are not enough for all the cases below.
However, I did check that it fits in less than a second.
There was a problem hiding this comment.
However, I did check that it fits in less than a second.
Okay, just keep in mind the ideal target is much less than 1 sec when possible. pytest -m 'not ultraslowtest' collects 3863 items for me so every bit counts. But it can be tough to optimize for ICA so we can live with it here 👍
At some point it would probably be good to do a minimal set of ica.fits and copy() the objects around implicitly via a new ica_raw_9comp fixture or so for example. But that can be for another PR :)
There was a problem hiding this comment.
That's fair, so done. It's easy enough to do for short test functions ;)
|
I'll update the docstring too, as per the comment of @hoechenberger on the issue. |
|
Thanks for going the extra mile (or 1.6 km) here @mscheltienne ! |
Closes #11653