-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Change IC names for ica.get_sources #2019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@teonlamont can you add a note? Maybe we can sell it as a bug fix ... |
|
So now channel is called ICA 000??? That's weird to me and not consistent with EEG 001 that we use elsewhere. I agree with @dengemann on this |
|
@agramfort you're keen on having a phone conversation? Teon is right that we do have usability issue. I'm not quite sure what would be the best. This one seemed the least resistance / effort path.
|
|
so you say we have channels starting at ICA 000 ? I am thinking like you
that you not manually select channels based on indices but use list.index
method.
|
yes, me, yes, you. Thinking of newcomers without MNE/Python and what they would expect by default. Once more, in viz functions by the ICA objects we plot indices. After conversion we plot channel names. We should find a way to reduce that ambiguity. |
|
conversion? sorry I am lost. Call me if it's easier.
|
|
if we go that way, we should update the name on the topo plot to reflect channels rather than indices. what is the naming convention when an IC has been zeroed out, do we manage the numeric of the zeroed out IC or do we just have an overall smaller set |
|
but the ICA object does not have ICA channels. |
|
Maybe we should deprecated .get_sources ;) |
|
honestly I am not sure here. @teonlamont was it that confusing after all? we should have soon with the GSOC an interactive selection of IC components. |
Not sure we will go too far in that direction. Exporting sources to raw is possible but not the primary desing. I'm also not sure what to do. Maye better documentation. Thinking about it once more the behavior is consistent with the API that you would expect by design from each object. |
|
the thing is I got mixed up with just looking at the ICs and their raw.plot correspondence. I was also just explaining the ICA with a colleague and it came up without me mentioning it when I should them a ECG artifact showing topo then the time course. |
The problem is, that you're using the API in a non-standard way. For the ICA object we have a function to show time-courses. |
|
And it is consitent with the topo. |
|
Maybe the cleaner thing would be to implement some interactive sources viewer, if this is really wanted. |
|
honestly. I did a lot of ICA in the last 2.75 years using our code. I actually never used the raw.plot for looking at things. |
then that means that the |
|
I used it to visualize the time course of the ECG before I knew there was an automated way to do so |
maybe that's right. But so far it is consistent with the objects it returns. I think it's more a documentation issue, I see it as an expert function. But there's also some historical aspect to it. It used to return numpy arrays, later we made it return objects. |
|
ica.plot_sources could use the raw viewer when passed raw inst. That would
avoid the conversion to raw instance with get_sources
|
|
I didn't even realize that was a feature. yeah, maybe an option to choose between the overlay and the raw viewer would be great and could alleviate the problem. |
|
I am voting to keep this for later and don't change any behavior for 0.9
here.
ok?
|
|
that's fine with me. I will open an issue to keep track of it |
closes #2000. The names for the IC in plot_topomap and the names in the
Rawobject afterica.get_sourcesdiffer. One solution is to name the channels as we are displayed in the topo for a clear and consistent nomenclature.