[MRG] Add copy and channel selection for a Layout object #12338
Conversation
mscheltienne
requested review from
larsoner,
drammock,
agramfort,
dengemann and
jasmainak
as code owners
mscheltienne
changed the title
mscheltienne
changed the title
| self.pos = self.pos[picks] | ||
| self.ids = self.ids[picks] | ||
| self.names = [self.names[k] for k in picks] | ||
| return self |
There was a problem hiding this comment.
did you check if this logic is not present elsewhere? I would be surprised if it's not.
There was a problem hiding this comment.
Yes, the only oneI found that would work is picks = _picks_to_idx(len(layout.names), picks) which picks on a number of channels. It's more limited and restrictive on the inputs than the logic above.
I do plan to propose a new channel selection API, to try to 1. clean-up all the redundant code and multiple pick functions in mne._fiff.pick and 2. open a public API for channel selection (#11913). Hopefully next week 😉
|
Yes, the only oneI found that would work is picks =
_picks_to_idx(len(layout.names), picks) which picks on a number of
channels. It's more limited and restrictive on the inputs than the logic
above.
I do plan to propose a new channel selection API, to try to 1. clean-up
all the redundant code and multiple pick functions in mne._fiff.pick
<https://github.com/mne-tools/mne-python/blob/main/mne/_fiff/pick.py> and
2. open a public API for channel selection (#11913
<#11913>). Hopefully next
week 😉
yes I think #11913 should be done first here. I would not have this complex
and common code here.
… Message ID: ***@***.***>
|
|
Agree, but I think #11913 will take time, thus I would first merge this PR with a TODO comment left in the codebase and a x-ref to replace the logic with whatever is defined in #11913. Especially as I do have someone at our site that could use this channel selection on a layout to improve his topographic plots. |
There was a problem hiding this comment.
@mscheltienne still want this one? Sorry it fell off the radar a bit. Seems good enough to go for me
|
It would be nice, yes :) |
|
Thanks @mscheltienne ! |
I had a report that
plot_evoked_topo(..., layout=layout, exclude=["M1", "M2"])was partially ignoring the exclusion. The plot still had the 2 empty M1/M2 channels because they were present in the layout.I added 2 methods to the
Layoutobject:copyandpick. Nothing fancy, beside that thepickmethod is not based on a function inmne._fiff.pick. I did not find one that was fitting all the needs for aLayoutinstance. Thepickmethod is now used in the plotting function.