Skip to content

Conversation

@marsipu
Copy link
Member

@marsipu marsipu commented Jun 3, 2021

Reference issue

For #9417

What does this implement/fix?

This is supposed to make the differentiation of multiple ICA-components displayed for evokeds easier by displaying them in different colors.

Questions

Are there more ICA-Categories than EOG/ECG which would be displayed simultaneously to consider?
In this draft I accounted for different categories by assigning a different colormap for each one.

@hoechenberger
Copy link
Member

hoechenberger commented Jun 3, 2021

Good start! I would differentiate categories by line style and components by color 💪

@marsipu
Copy link
Member Author

marsipu commented Jun 3, 2021

Good start! I would differentiate categories by line style and components by color 💪

Thank you for the idea. How big is the possible number of different categories I should account for?

@hoechenberger
Copy link
Member

So far I've only seen two in real life (ECG and EOG), not sure if we even support anything else right now? 😅

@marsipu marsipu force-pushed the ica_evokeds_color branch from 8611cff to 412b085 Compare June 9, 2021 15:23
@marsipu marsipu marked this pull request as ready for review June 9, 2021 22:33
@marsipu marsipu changed the title WIP: multiple colors for ica.plot_sources(evokeds) ENH: multiple colors for ica.plot_sources(evokeds) Jun 9, 2021
@marsipu marsipu force-pushed the ica_evokeds_color branch from 15b617f to 271f2c1 Compare June 10, 2021 09:04
@hoechenberger
Copy link
Member

Thanks @marsipu! Can you touch one of the ICA tutorials so we can actually see the effects in the generated report?

https://mne.tools/stable/auto_tutorials/preprocessing/40_artifact_correction_ica.html?highlight=ica#using-a-simulated-channel-to-select-ica-components might be a good place!

By "touching" I really mean, simply changing a tiny little thing, e.g. improving phrasing somewhere, to enforce re-rendering of the tutorial

@marsipu
Copy link
Member Author

marsipu commented Jun 10, 2021

Where do I find the generated report?

@drammock
Copy link
Member

Screenshot_2021-06-10 ENH multiple colors for ica plot_sources(evokeds) by marsipu · Pull Request #9444 · mne-tools mne-pyt

@marsipu
Copy link
Member Author

marsipu commented Jun 10, 2021

Thank you! I also created this gist to test multiple component-categories.

@hoechenberger
Copy link
Member

hoechenberger commented Jun 10, 2021

It's working! 🥳

https://29347-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/preprocessing/40_artifact_correction_ica.html#using-a-simulated-channel-to-select-ica-components

I'm not a big fan of the color map though, can we try to see what it looks like e.g. with tab10?

@agramfort
Copy link
Member

anyone feel free to MRG if happy. thx @marsipu

@drammock
Copy link
Member

drammock commented Jun 10, 2021

I'm not a big fan of the color map though, can we try to see what it looks like e.g. with tab10?

I tend to agree with @hoechenberger here, that we should stick with MPL default color choices (which in this case means tab10) unless we have a good reason not to. I also found the gist example a little... overwhelming; it seemed to me difficult to keep track of which ones were ECG and which were EOG, etc. I think that particular visualization would be better if color (rather than linestyle) were held fixed within a component type (so, ECG component 1 is blue solid, ECG 2 is blue dashed; EOG1 is orange solid, EOG2 is orange dashed; etc). However, I acknowledge that this is a nitpick verging on bikeshedding, especially since I don't think it's all that common to view multiple component types at once (?). So in that light maybe it is better to [hold linestyle fixed and vary color] within a given component type, since the most common use case is to show 1-3 components of the same kind (EOG, ECG, etc) and nothing else. But having talked myself out of the suggestion to switch how color/style are varied, I still think tab10 is a better choice than rainbow.

@marsipu
Copy link
Member Author

marsipu commented Jun 11, 2021

@drammock thank you for the feedback. I agree that especially the example from the gist doesn't proof the point of differentiating the components more easily. I also think that showing multiple component-types together is probably more rare than multiple components from the same type. At the moment, I just use the 4 named matplotlib linestyles, so displaying more than 4 different components with the same color but different linestyles would then lead to the original problem that components could not be differentiated (these could of course expanded with custom linestyles, but that would maybe not help much in terms of differentiation). So as you suggest I would keep it the way it is currently implemented and change the colormap to tab10 (for excluded_components>10 this would result in equal colors again, but so many excluded components are probably unlikely, right?).

@marsipu
Copy link
Member Author

marsipu commented Jun 11, 2021

Colormap tab10:
grafik

@hoechenberger
Copy link
Member

This looks fantastic!

@drammock drammock merged commit a0d5d76 into mne-tools:main Jun 11, 2021
@drammock
Copy link
Member

in it goes! Thanks @marsipu

@marsipu marsipu deleted the ica_evokeds_color branch June 11, 2021 14:32
@marsipu
Copy link
Member Author

marsipu commented Jun 11, 2021

Thank you:). Somehow #9417 wasn't closed

@hoechenberger
Copy link
Member

Thanks @marsipu

hoechenberger added a commit to hoechenberger/mne-python that referenced this pull request Oct 9, 2021
This fixes a regression that I believe was introduced via  mne-tools#9444
larsoner pushed a commit that referenced this pull request Oct 11, 2021
…9823)

This fixes a regression that I believe was introduced via  #9444
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants