Mne-python: API: picks as list of string

[kingjr]

Picking a particular channel is currently relatively complex for new users.

It either involves pythonic tricks:

picks = [ii for ii, ch in enumerate(epochs.ch_names) if ch == 'mychan']
epochs.plot(picks=picks)

Or master of MNE's functions:

picks = mne.pick_channels(evoked.info. [`my_chan`])
epochs.plot(picks=picks)

Proposal 1

:+1: 0
Since picks is always referring to a list of int and ch_names is always a list of str, I propose that we accept picks ot be a list of str:

evoked.plot(picks=list(range(10)))
epochs.plot(picks=['my_chan'])

Proposal 2

:+1: 0

Accepting the parameter picks to be a list of int (for indexing), a list of str (to find channel names) or a dict (to quickly select channel types).

evoked.plot(picks=list(range(10)))
evoked.plot(picks=['MEG 1132', 'MEG 1022'])
evoked.plot(picks=dict(grad=True, eeg=False))

• advantage: no deprecation, simplification of channel picking
• disadvantage: keep clunky pick_type API

Proposal 3

:+1: 0

Same as proposal 2, but change pick_types to accept a types key that would accept list of str. This would allow

evoked.plot(picks=list(range(10)))
evoked.plot(picks=['MEG 1132', 'MEG 1022'])
evoked.plot(picks=dict(grad=True, eeg=False))
evoked.plot(picks=dict(types=('eeg', 'mag')))
mne.pick_types(info, types=('eeg', 'mag'))

• advantage: easier to loop across channel types, no deprecation
• disadvantage: more than one way to do pick_types

Proposal 4:

:+1: 4 (@larsoner, @kingjr, @nbara, @choldgraf)

Same as proposal 1, but automatically detect if str refer to ch_names of ch_types.

evoked.plot(picks=list(range(10)))
evoked.plot(picks=['MEG 1132', 'MEG 1022'])
evoked.plot(picks=('eeg', 'meg'))

• inconvenient: risk of conflict between channel names and channel types
• advantage: easier to loop across channel types

[dengemann]

@kingjr I had thought about this for long time and am glad you bring it up.
I think it can be a good idea. It will be unproblematic whenever the
function picks are passed to has access to .info.
I sometimes thought we should have written mne.pick_types that way to
accept a list of channel keys. But I would not suggest to depracate it.
Maybe a keys argument could be added there.
On Fri, 18 Aug 2017 at 09:25, Jean-Rémi KING [e-mail]
wrote:

Picking a particular channel is currently relatively complex for new users.

It either involves pythonic tricks:

picks = [ii for ii, ch in enumerate(epochs.ch_names) if ch == 'mychan']
epochs.plot(picks=picks)

Or master of MNE's functions:

picks = mne.pick_channels(evoked.info. [my_chan])
epochs.plot(picks=picks)

Since picks is always referring to a list of int and ch_names is always a
list of str, I propose that we accept picks ot be a list of str:

epochs.plot(picks=['my_chan'])

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502, or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fivoEudkx2qgdaG_9TUCtDEreRSCfks5sZTyAgaJpZM4O7MZP
.

[kingjr]

@dengemann For pick_types we could accept picks to be a dict

[dengemann]

I don't yet see what you have in mind. The function takes keyword arguments
for channel types. It is annoying that you cannot just loop over types but
have to. Create bool flags and then deal with meg that can be 'mag', 'grad'
or True/False.
If you could do:

pick_types(info, keys=('mag', 'eeg', 'eog'))

That would simplify lots of code.

On Fri, 18 Aug 2017 at 09:34, Jean-Rémi KING [e-mail]
wrote:

@dengemann https://github.com/dengemann For pick_types we could accept
picks to be dict

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323282298,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fipb5WZB4Oiuk_6H0CbGWQDHtM8dDks5sZT6MgaJpZM4O7MZP
.

[kingjr]

Proposal 2

Sorry, I meant accepting the parameter picks to be a list of int (for indexing), a list of str (to find channel names) or a dict (to quickly select channel types).

evoked.plot(picks=list(range(10)))
evoked.plot(picks=['MEG 1132', 'MEG 1022'])
evoked.plot(picks=dict(grad=True, eeg=False))

EDIT: Copied to top

[dengemann]

Yes. I think we are discussing complementary ideas. I like the list of
types notation more as it is far more concise. It could not only simplify
passing picks but creating them. In some situations you would still need
the full integer arrays.
On Fri, 18 Aug 2017 at 09:53, Jean-Rémi KING [e-mail]
wrote:

Sorry, I meant accepting the parameter picks to be a list of int (for
indexing), a list of str (to find channel names) or a dict (to quickly
select channel types).

evoked.plot(picks=dict(grad=True, eeg=False))
evoked.plot(picks=range(10))
evoked.plot(picks=['MEG 1132', 'MEG 1022'])

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323285866,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fiveZMMqfJzTETEgCaaZeyMOOh9Qlks5sZULmgaJpZM4O7MZP
.

[kingjr]

I see. However, the issue is that a list of types would conflict with a list of names.

evoked.plot(picks=['MEG 1132', 'MEG 1022'])
evoked.plot(picks=['mag', 'grad'])

I'm thus in favor of Proposal 2, i.e. accepting list of string and dict.

I'll update the above description.

[larsoner]

@kingjr I updated your proposal to to be Py3k-friendly, and to reorder with list(range(...)) first (since it's what we have now)

[larsoner]

It does seem like proposal 2 would just require a ~6-line private function anytime we use picks, so it's not so bad from an implementation standpoint. And from a user standpoint it's pretty simple.

It does have overlap, e.g. inst.pick_types(['EEG 001', 'EEG 002']) will be the same as inst.pick_channels(['EEG 001', 'EEG 002']), but I can live with that.

@kingjr if your sense is that this will reduce pain substantially for new users, +1 from me

[dengemann]

I would still prefer proposal 1 (list of channel types). It would allow me
to kill lots of code in virtually any project. I don't think we would ever
hit a case where channel names and type names are identical. For updating
mne.pick_types along these lines we would anyways have to add a new keyword.
On Fri, 18 Aug 2017 at 13:56, Eric Larson [e-mail] wrote:

It does seem like proposal 2 would just require a ~6-line private function
anytime we use picks, so it's not so bad from an implementation
standpoint. And from a user standpoint it's pretty simple.

It does have overlap, e.g. inst.pick_types(['EEG 001', 'EEG 002']) will
be the same as inst.pick_channels(['EEG 001', 'EEG 002']), but I can live
with that.

@kingjr https://github.com/kingjr if your sense is that this will
reduce pain substantially for new users, +1 from me

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323334482,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fioDVNKQm7ofrms7znsfzr1d0jV-Gks5sZXv3gaJpZM4O7MZP
.

[kingjr]

I would still prefer proposal 1 (list of channel types). It would allow me to kill lots of code in virtually any project.

Can you describe in what way Proposal 2 wouldn't?

I don't think we would ever hit a case where channel names and type names are identical.

This would still require a "smart" guess; not safe IMO.

For updating mne.pick_types along these lines we would anyways have to add a new keyword.

Why? We wouldn't need to update pick_types in any of these proposal

[larsoner]

Why? We wouldn't need to update pick_types in any of these proposal

Agreed, we would just update any function that has picks argument. Instead of taking only list of int, take the other things, too. pick_types and pick_channels would remain the same.

[dengemann]

I would independently from these proposals like to get rid of the dict /
pick_types api because it is too irregular and makes looping over channel
types a pain.
In terms of elegance ('mag', 'eeg') is much more convenient then
dict(meg='mag', eeg=True).

On Fri, 18 Aug 2017 at 14:12, Eric Larson [e-mail] wrote:

Why? We wouldn't need to update pick_types in any of these proposal

Agreed, we would just update any function that has picks argument.
Instead of taking only list of int, take the other things, too. pick_types
and pick_channels would remain the same.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323337394,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fio2M4YHkN9w32M6DBcPQV0NxSvkBks5sZX-9gaJpZM4O7MZP
.

[kingjr]

I agree; although for this we can propose an orthogonal PR where mne.pick_types accepts a list of str

mne.pick_types(info, types=('mag', 'grad', 'eog'))

[dengemann]

Sounds like a good plan. Once you have picks you can use it downstream the
old way.
On Fri, 18 Aug 2017 at 14:43, Jean-Rémi KING [e-mail]
wrote:

I agree; although for this we can propose an orthogonal PR where
mne.pick_types accepts a list of str

mne.pick_types(info, types=('mag', 'grad', 'eog'))

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323343606,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fipWp45r_uQqwp1zLWXQ56NJi6-dHks5sZYcJgaJpZM4O7MZP
.

[choldgraf]

I'm +1 with

mne.pick_types(info, types=('mag', 'grad', 'eog'))

I think this is much more descriptive than having a gigantic list of true/false.

I'm also +1 for allowing lists of strings. I'm a little hesitant to allow for dict(eeg=True) since I agree that is a clunky API already.

What if instead of dict(grad=True.., we allowed dict(types=['eeg', 'meg', ...])?

[larsoner]

I think that picks=dict(types=['eeg']) is about the same clunkiness as picks=dict(eeg=True).

Thinking about it more I actually don't mind picks=['eeg', 'meg']. I think that collisions will be extremely rare. In those cases where they do collide, we can raise an error about ambiguity and tell people to use picks=pick_channels or picks=pick_types explicitly.

[choldgraf]

I'm +1 with that. Just do something like

PICK_TYPES = ['eeg', 'meg', 'ecog', 'blahblah']
if any(any(ii == np.array(data.ch_names)) for ii in PICK_TYPES):
    warn('wtf dude use better names')

[larsoner]

It will be slightly more complicated because we need to check if all are channels, or all are types, and raise an error if it doesn't fit exactly one of those criteria, but yes something like that.

[kingjr]

ok, I'm updating the proposal sheet, wait for what @agramfort think to make any attempt PR, as API change isn't the least controversial to do.

[larsoner]

We just need to ensure he first sees that #3842 was merged so that he's in a good mood when reading this proposal

[choldgraf]

I have another proposal: what if we allow users to supply a dataframe to the picks argument. This dataframe would contain a single column, 'data', and each row of this column would be another dataframe that would contain a list of Series objects, one for each data type to be plotted.

[choldgraf]

also we can deprecate the picks argument and make it pick_pandas

[kingjr]

@choldgraf I actually thought of this, it'd be make so much more sense.... accessing the channel position, type etc would be so simple...

[larsoner]

Only if we can do a dataframe of dataframes, each with a single column for each channel type

Just so we're clear, my vote is for Proposal 4. I'm going to update the proposal to put my +1 there, everyone else feel free to do the same (people without permission to change, leave a comment and we'll update)

[larsoner]

@nbara you had an overall :+1: feel free to have another look at the top comment if you have time

[choldgraf]

so simple!

(in seriousness, I'm +1 with proposal 4 too)

[kingjr]

Only if we can do a dataframe of dataframes, each with a single column for each channel type

?

epochs.info['chs'] = pd.DataFrame(dict(name='MEG 001', type='mag', pos=xyz, ..., ...))
# find chan type
epochs.ch_names.query('type=="mag"')
# find specific combinations
epochs.ch_names['name'].apply(lambda x: 'meg 10' in x)

[choldgraf]

@kingjr you aren't supposed to expose our secret plot to slowly introduce pandas into everything in MNE!

[larsoner]

@kingjr that was a joke, using Pandas for this sort of thing seems way overkill

[dengemann]

Guys let's do one revolution at a time. Overcoming the picks dict would
already be a great step for mnekind.
On Fri, 18 Aug 2017 at 19:27, Eric Larson [e-mail] wrote:

@kingjr https://github.com/kingjr that was a joke, using Pandas for
this sort of thing seems way overkill

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323413336,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fih68WleBLfhj2JjPV5ZjlJ2824rsks5sZcmQgaJpZM4O7MZP
.

[dengemann]

Thinking of mne.pick_types I think we never had a single example supporting
meg/eeg/eog/ etc. as positional arguments.

We could indeed dare to make types the 2nd argument:

picks = mne.pick_types(raw.info, ('mag', 'eeg'))

That would be simply beautiful.
On Fri, 18 Aug 2017 at 19:29, Denis-Alexander Engemann <
[email protected]> wrote:

Guys let's do one revolution at a time. Overcoming the picks dict would
already be a great step for mnekind.
On Fri, 18 Aug 2017 at 19:27, Eric Larson [e-mail]
wrote:

@kingjr https://github.com/kingjr that was a joke, using Pandas for
this sort of thing seems way overkill

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323413336,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fih68WleBLfhj2JjPV5ZjlJ2824rsks5sZcmQgaJpZM4O7MZP
.

[choldgraf]

IMO that is the most conceptually simple way to pick types, +1

[dengemann]

That wouldn't even necessitate deprecation of anything I would say.
@choldgraf @kingjr @larsonser
On Fri, 18 Aug 2017 at 19:38, Chris Holdgraf [e-mail]
wrote:

IMO that is the most conceptually simple way to pick types, +1

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323415822,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fijkyT3LOn0KyOZ35BV3dW1KzP2Ycks5sZcv5gaJpZM4O7MZP
.

[nbara]

I'm +1 for Proposal 4 as well!

[agramfort]

ok I coming late here but my first reaction is that "There should be one-- and preferably only one --obvious way to do it."

regarding your proposal 4 that seems to satisfy most of you:

evoked.plot(picks=list(range(10)))  # already works

evoked.plot(picks=['MEG 1132', 'MEG 1022'])  # can be done with:
evoked.copy().pick_channels(['MEG 1132', 'MEG 1022']).plot()

evoked.plot(picks=dict(grad=True, eeg=False))  # can be done with:
evoked.copy().pick_types(meg='grad', eeg=False).plot()

by putting the picking in the data container we don't need to have a complex pick logic in every viz or computation function.

thoughts?

[larsoner]

ok I coming late here but my first reaction is that "There should be one--
and preferably only one --obvious way to do it."

I agree as a general principle, and this does clearly violate it. The
question in my mind is: does the added simplicity and meeting of users
expectations justify it?

by putting the picking in the data container we don't need to have a
complex pick logic in every viz or computation function.

It's still a bit harder. Most users probably want picks to do what 4
proposes, and have some disappointment when it doesn't.

Well the complex logic wouldn't exactly be in every function. It would be
in a single private function (that gets called by each function). But
conceptually this replaces one DRY function that should already exist but
probably doesn't (to validate picks are list-like of int and are all valid
values).

[dengemann]

@agramfort think of how our proposal would help looping over channel types
to get picks. At least for pick_types. You just loop over ('meg', 'eeg')
etc.

The dict of bool / string demanded by its current API is mildly spoken a
PITA. You need to make exceptions for "meg" which can be
True/False/"grad"/"mag". Then it is not parsimonious. Why eeg=True if it
can just be "eeg".
If we could overcome for once our purist API change culture we would make
certainly 99% of all users significantly more happy.
Or differently put the one way to do it that we currently suggest is not
the right / best one.

Please think of it twice.
On Sat, 19 Aug 2017 at 14:25, Eric Larson [e-mail] wrote:

ok I coming late here but my first reaction is that "There should be one--
and preferably only one --obvious way to do it."

I agree as a general principle, and this does clearly violate it. The
question in my mind is: does the added simplicity and meeting of users
expectations justify it?

by putting the picking in the data container we don't need to have a
complex pick logic in every viz or computation function.

It's still a bit harder. Most users probably want picks to do what 4
proposes, and have some disappointment when it doesn't.

Well the complex logic wouldn't exactly be in every function. It would be
in a single private function (that gets called by each function). But
conceptually this replaces one DRY function that should already exist but
probably doesn't (to validate picks are list-like of int and are all valid
values).

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323520147,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0fivNHYzXcU_GvGy3w4qIyXLSd6tyRks5sZtRWgaJpZM4O7MZP
.

[agramfort]

how would you write this:

picks = pick_types(raw.info, meg=False, eeg=True, stim=False, eog=False,
                   exclude='bads')

epochs = Epochs(raw, events, event_id, tmin, tmax, proj=False,
                picks=picks, baseline=None, preload=True,
                verbose=False)

would we deprecate some pick_ methods or functions to avoid duplications?

[choldgraf]

wouldn't you just do:

Epochs(raw, events, event_id, tmin, tmax, ..., picks=['eeg'], ...

or if you wanted a subset of channels:

Epochs(raw, events, event_id, tmin, tmax, ..., picks=['ch_01', 'ch_03'])
?

[agramfort]

this would exclude bads automatically?
don't you make a difference between ('eeg') and ['eeg'] ie tuple for type
and list of ch names?

[choldgraf]

ah I see what you mean - in that case I think it'd be fine to include an exclude kwarg people can use in Epochs...it seems like a natural thing to do.

re: type vs. list of names, I think @larsoner's thought was that it would be rare enough not to be a problem and that we could handle it with good warnings or errors. That said, I agree it feels clunky

[larsoner]

In practice I don't expect collisions, and when they do happen it's easy to
redirect to functions.

I'd say behave as exclude='bads' and if you want something more advanced
(e.g., bads or ignore_ref non-defaults) use the functions. -1 on adding
extra kwargs to deal with all of these cases.

In my mind this is about improving convenience and removing barriers to
entry for minimal maintenance and implementation cost, not replacing the
existing, more complete API.

[nbara]

@agramfort If I may, as a relatively new user, I find channel picking in MNE to be extremely unintuitive. Syntaxes like these in particular seem very convoluted to me :

evoked.copy().pick_channels(['MEG 1132', 'MEG 1022']).plot()
evoked.copy().pick_types(meg='grad', eeg=False).plot()

I appreciate that an API change is not insignificant, but in this case I would find it most welcome.

(just my 2¢)

[agramfort]

@nbara would you suggest to also simplify these lines:

evoked.copy().pick_channels(['MEG 1132', 'MEG 1022'])
evoked.copy().pick_types(meg='grad', eeg=False)

ie without the plot? or it's the fact that you need to pick data before
plotting?

[dengemann]

@agramfort

picks = pick_types(raw.info, types=('eeg',), exclude='bads')

epochs = Epochs(raw, events, event_id, tmin, tmax, proj=False,
                picks=picks, baseline=None, preload=True,
                verbose=False)

We would not need to deprecate anything.
Once types is not None, all other flags will be ignored.
Already updating this function would clean up the code as it allows you to get picks very easily:

for ch_type in ('mag', 'grad', 'meg', 'eeg'):
    picks = mne.pick_types(raw.info, types=(ch_type,))
    # or
    picks = mne.pick_types(raw.info, (ch_type,))
    pass

[agramfort]

What I am thinking is that we could have a single pick method

evoked.pick(picks, channels, types)

so what @nbara suggests would be written:

evoked.copy().pick(channels=['MEG 1132', 'MEG 1022']).plot()
evoked.copy().pick(types=['grad']).plot()

so the semantic of picks stays picks=list of int

[dengemann]

Which would be compatible with the upgrade of mne.pick_types which always
returns array of int. Keep in mind that we need picks in data that don't
have info, pure numpy.
On Mon, 21 Aug 2017 at 11:45, Alexandre Gramfort [e-mail]
wrote:

What I am thinking is that we could have a single pick method

evoked.pick(picks, channels, types)

so what @nbara suggests would be written:

evoked.copy().pick(channels=['MEG 1132', 'MEG 1022']).plot()
evoked.copy().pick(types=['grad']).plot()

so the semantic of picks stays picks=list of int

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323698389,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0figcYHbqJ-eMnE7vkf5ZDK2QbNQgYks5saVG7gaJpZM4O7MZP
.

[agramfort]

would we still need to refactor pick_types?

you envision a pb with Covariance class?

[dengemann]

I need mne.pick_types all the time. But taking what you and @nbara suggest
to the next level we could have:

mne.pick(info, types, channels=None, exclude='bads', include=None)

On Mon, 21 Aug 2017 at 11:59, Denis-Alexander Engemann <
[email protected]> wrote:

Which would be compatible with the upgrade of mne.pick_types which always
returns array of int. Keep in mind that we need picks in data that don't
have info, pure numpy.
On Mon, 21 Aug 2017 at 11:45, Alexandre Gramfort [e-mail]
wrote:

What I am thinking is that we could have a single pick method

evoked.pick(picks, channels, types)

so what @nbara suggests would be written:

evoked.copy().pick(channels=['MEG 1132', 'MEG 1022']).plot()
evoked.copy().pick(types=['grad']).plot()

so the semantic of picks stays picks=list of int

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323698389,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0figcYHbqJ-eMnE7vkf5ZDK2QbNQgYks5saVG7gaJpZM4O7MZP
.

[agramfort]

@nbara please let me know what you think

[nbara]

What I am thinking is that we could have a single pick method

Yes that would be good IMO !

or it's the fact that you need to pick data before plotting?

Yes this is confusing too (but maybe another issue)

@dengemann's suggestion is definitely an improvement in my view

[larsoner]

I don't like these proposals as much as the existing number 4.

The unified pick function is ambiguous / less obvious to me. For example,
what happens when I pass types and names? Is it and-like or or-like?

For the unified pick as an alternative (even though from an API perspective
it's orthogonal) to the picks string list, functions that take a picks
argument (like Epochs) are still overly complex for most use cases. People
will still reasonably wish that they could directly pass a list of string
(as channels or types), and we will continue to disappoint them by not
allowing it.

The copy-pick-operate proposal can be (very) memory intensive compared to
the picks-argument equivalent, too. This can especially be three case in
the common raw-filter-epoch loop.

It's also not the same -- something like raw.filter can only reasonably be
done with picks-argument way. So pursuing copy-pick-operate as the ideal
pattern is a bit of a non-starter from my end.

[agramfort]

hum... the world is against me. Yet:

"il y a que les imbéciles qui changent pas d'avis"

so every function/method that has picks as parameter now will first call

picks = check_picks(info, picks)

where check_picks would be in utils.py ?

@larsoner if we do this we should do it without increasing test time.

[jasmainak]

I honestly find the existing picks API quite intuitive and well designed, but what I find missing is a clear documentation or tutorial.

For me, picks is nothing but a list of indices and all the pick_xxx functions are helpers to compute these indices. If you are using a scripting language like Matlab or Python, indexing is one of the first thing you learn.

If you change the semantics of what picks is (e.g., in Proposal 4), this will require a change in the code not just in code but also in other repos (like autoreject). [@dengemann up for another PR? :P ]

Please consider carefully the implications of this change

[dengemann]

My minimum to be happy being able to use an mne function to loop over
channel types and get corresponding integer picks. Maybe we simply have to
rehibilitate mne.io.pick.channel_indices_by_type. It is not as beautiful as
"fixing" pick_types but maybe does the job. Maybe we could improve it.

Mainak for autoreject I was recently cursing a lot about pick_types. A
better api here would allow us to remove lines over there.
On Mon, 21 Aug 2017 at 15:33, Mainak Jas [e-mail] wrote:

I honestly find the existing picks API quite intuitive and well designed,
but what I find missing is a clear documentation or tutorial.

For me, picks is nothing but a list of indices and all the pick_xxx
functions are helpers to compute these indices. If you are using a
scripting language like Matlab or Python, indexing is one of the first
thing you learn.

If you change the semantics of what picks is (e.g., in Proposal 4), this
will require a change in the code not just in code but also in other repos
(like autoreject). [@dengemann https://github.com/dengemann up for
another PR? :P ]

Please consider carefully the implications of this change

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-323744927,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0firUG1wxCuNZg3qsdahAhYjNOXFMAks5saYcMgaJpZM4O7MZP
.

[nbara]

@jasmainak Not sure I agree with you.

Like many others I'm guessing, I spend most of my analysis time doing exploratory work, i.e. quickly looking at my data to get a "feel" of how they look, or check that e.g. an basic ERP is here or not.

And to do this the current system is particularly unintuitive. For instance I'll want to look at channel Oz, or Cz, and I don't care what index they correspond to (esp. given that these indices might change depending on the layout), ideally I'd just want to do evoked.plot('Cz'), and then quickly see I I get something similar in another channel evoked.plot('Cz'). Then I might wan to try new filter settings on all my channels before plotting again. This takes quite a lot of keystrokes currently. Not to mention that the process of copy-picking before plotting is just... weird.

[agramfort]

what I realize with this conversation is that while EEG users know the name
of their channels MEG people don't.... they are just stupid names and
numbers.

@nbara do you know this PR:
https://github.com/mne-tools/mne-python/pull/4221

[choldgraf]

*and ECoG people have 0 ability to predict their channel names ahead of time

[larsoner]

If you change the semantics of what picks is (e.g., in Proposal 4), this
will require a change in the code not just in code but also in other repos
(like autoreject). [@dengemann https://github.com/dengemann up for
another PR? :P ]

No, proposal 4 adds functionality, it does not remove existing
functionality of None or list of int, i.e. it is backward compatible,
unless you have some code that relies on errors being thrown for non-int
lists.

I don't expect any appreciable test time increase from this either.

[jasmainak]

@nbara have you used evoked.plot_topo? It seems to be exactly for your requirement.

[jasmainak]

No, proposal 4 adds functionality, it does not remove existing functionality of None or list of int, i.e. it is backward compatible, unless you have some code that relies on errors being thrown for non-int lists.

I think we would have to handle non-int lists as you correctly anticipate. When it is int, it's really easy to index matrices without thinking twice. You're probably proposing that picks is internally handled still as an int list but I guess that would need additional code ...

[nbara]

@jasmainak No I wasn't aware of plot_topo, thanks ! Does it work for TF data?
@agramfort #4221 looks very neat indeed!

MEG people don't.... they are just stupid names andnumbers.

O_O I used to work with MEG and I knew exactly where my channels were (on a CTF system, I used to plot a bunch of channels together using wildcards such as MRT* to get all right temporal sensors etc.).
As for ECoG, well yes... But wouldn't you just have some kind of map/image showing electrode implantation and use that to look at your activations for each patient ? I mean, it's not about knowing the location in advance, in some cases it's just more convenient to plot stuff by some kind of label instead of by number. Or maybe that's just me :)

Anyway, I don't mean to pollute this thread too much. Still in favour of Proposal 4.

[jasmainak]

@jasmainak No I wasn't aware of plot_topo, thanks ! Does it work for TF data?

yep, take a look here :)

[agramfort]

ok so can we try a first draft of this starting with viz functions?

[larsoner]

I can give a proof of concept a shot this week unless @kingjr wants to do it

[kingjr]

I m off this week, so go ahead, but I reckon we should aim to merge it
after the release to allow us to get a good API check over the next months.

On Tuesday, 22 August 2017, Eric Larson [e-mail] wrote:

I can give a proof of concept a shot this week unless @kingjr wants to do
it

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/4502#issuecomment-324006605,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEp7DDflLB6Qax5m6cwQTtbL10KPdD-Sks5sasTMgaJpZM4O7MZP
.