MRG, ENH: Support n_col keyword in ica.plot_score

[bloyl]

In playing with the recent ica ref channels artifact rejection (#7810), I'd sometimes have 4-5 score plots in one column, which I couldn't see.

This PR adds a parameter ('n_col') to 'ica_plot_scores' to use a grid with multiple columns.

There is a question about the following from the test.
ica.plot_scores([0.3, 0.2], axhline=[0.1, -0.1], labels='ecg')
it fails on this PR and is currently commented out.

On master it passed but did not make a plot, because:

if labels == 'ecg':
        labels = [label for label in ica.labels_ if label.startswith('ecg/')]

returned labels = [] and nothing was plotted.

I don't know enough about the ica labeling so someone should advice what the correct behavior is and how to test it.

[bloyl]

@larsoner or @dengemann could either of you take a look at the commented out test? I'm not sure what it was trying to test.

[bloyl]

I just pushed a change to the function that 1) Lets the test pass 2) Does what I think was expected before I'm good to MRG on my end if everyone is happy.

…

On Tue, May 26, 2020 at 10:52 AM Eric Larson ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In mne/viz/tests/test_ica.py <#7825 (comment)>: > ica.plot_scores([0.3, 0.2], axhline=[0.1, -0.1], labels='foo') ica.plot_scores([0.3, 0.2], axhline=[0.1, -0.1], labels='eog') - ica.plot_scores([0.3, 0.2], axhline=[0.1, -0.1], labels='ecg') + # ica.plot_scores([0.3, 0.2], axhline=[0.1, -0.1], labels='ecg') one solution would be to first look for nested labels ('ecg/') and if none are found look for non-nested ('ecg') I have no idea if the labeling is supposed to work this way, it would require digging into the code... Maybe just add: ica.labels_['ecg/0/bar'] = 1 ? This should allow it to plot — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7825 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKTXHKQRU4WSDF2S75KBZLRTPJT5ANCNFSM4NJVPC2Q> .

[larsoner]

Just a minor nitpick/idea, then +1 for merge from me!

[larsoner]

Thanks for working to appease codecov :)

[agramfort]

Thx @bloyl