Back to the main page.

Bug 3225 - there are two copies of ft_channelselection

Status CLOSED FIXED
Reported 2016-12-20 10:38:00 +0100
Modified 2017-01-17 11:29:43 +0100
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P5 normal
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also:

Robert Oostenveld - 2016-12-20 10:38:01 +0100

one at fieldtrip/ft_channelselection.m and one at fieldtrip/utilities/ft_channelselection.m No idea how that happened. But they have diverged. I will merge the functionality and then remove the one in fieldtrip main.


Robert Oostenveld - 2016-12-20 11:01:45 +0100

There are changes that relate to 'megplanar' 'itab28' 'itab28_old' 'neuromag122_combined' 'neuromag306_combined' which I all merged. There is also a change related to the use of ^, where Jorn made this change in one exemplar https://github.com/fieldtrip/fieldtrip/commit/c243a966f8bac9c51c0dcd184f5ca39ba090a81c and Arjen made this change in the other exemplar https://github.com/fieldtrip/fieldtrip/commit/3920c08506c1e8a9479d27aa1043d4b8058e0fa2#diff-475eb1d25b73b5e91c9ab706ce30f259


Robert Oostenveld - 2016-12-20 11:02:04 +0100

mac011> git commit utilities/ft_channelselection.m [bug3225-channelselection b072eff] add old itab systems, copied over from fieldtrip/ft_channelselection. See http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3225 1 file changed, 23 insertions(+), 23 deletions(-)


Robert Oostenveld - 2016-12-20 11:03:05 +0100

mac011> git commit -a [bug3225-channelselection c4e9543] removed the ft_channelselection frunction from the main directory, the one to use is in utilities. See http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3225 1 file changed, 492 deletions(-) delete mode 100644 ft_channelselection.m


Robert Oostenveld - 2016-12-20 11:05:53 +0100

@arjen and @jorn, could you check (and settle among yourselves) whether the desired behavior of the ^ character works? To me it seems that the functionality added by Arjen works (that is also the one I left in place): >> ft_channelselection('^bc', {'abc', 'bc'}) ans = {'bc'} But I am not sure what the specific change of Jorn intended to achieve...


Jörn M. Horschig - 2016-12-20 11:25:30 +0100

Hi Robert and Arjen, please check test_ft_channelselection (last test). I added the '^' to account for bug 3135. Test case: desired = {'Cz','C3'}; label = {'Cz','FCz','FC3'}; sel = ft_channelselection(desired, label); The problem with this particular case is that FC3 wass detected as a desired channel, while it should not be selected. With Arjen's solution, this also fails. The solution I implemented does account for this and does only select channel Cz but not FC3.


Jörn M. Horschig - 2016-12-20 11:26:49 +0100

btw, I am not sure why the ^ should be added in front. It's more intuitive to me to add a * in front if you want to select FC3 in my example (i.e. desired = '*FC3')


Jörn M. Horschig - 2016-12-20 11:27:15 +0100

(In reply to Jörn M. Horschig from comment #6) desired = '*C3' of course


Robert Oostenveld - 2016-12-20 11:45:57 +0100

(In reply to Jörn M. Horschig from comment #7) I tend to agree. The interface (towards the user) tries to use wildcards, not regular expressions. Wildcards are widely understood. The ^ is a regexp syntax. I now also see that we have a test_ft_channelselection, and that Jorns example is included there. I have extended it with this ---------------------------------------------------------------------- %% test http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3135 desired = {'Cz','C3'}; label = {'Cz','FCz','FC3'}; sel = ft_channelselection(desired, label); assert(all(ismember(sel, desired)), 'not all selected channels were desired'); %% test http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3225#c7 sel = ft_channelselection('Cz', label); assert(numel(sel)==1) % only Cz itself sel = ft_channelselection('*Cz', label); assert(numel(sel)==2) % anything that ends with Cz sel = ft_channelselection('F*', label); assert(numel(sel)==2) % anything that starts with F sel = ft_channelselection('*3*', label); assert(numel(sel)==1) % anything with a 3 in it sel = ft_channelselection('*C*', label); assert(numel(sel)==3) % anything with a C in it ---------------------------------------------------------------------- and I have switched to Jorns code to deal with the wildcards. Agreed Arjen? mac011> git commit -a [master deeaea5] switch to Jorns code for channel selection with wildcard at the start, extended test script. See http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3225 2 files changed, 21 insertions(+), 8 deletions(-)


Arjen Stolk - 2017-01-03 00:15:06 +0100

Agreed. Nice work, see: >> ft_channelselection('*FG*', hdr.label) ans = 'FG1' 'FG2' 'FG3' 'FG4' 'FG5' 'FG6' 'FG7' 'FG8' 'FG9' 'FG10' 'FG11' 'FG12' 'FG13' 'FG14' 'FG15' 'FG16' 'FG17' 'FG18' 'FG19' 'FG20' 'FG21' 'FG22' 'FG23' 'FG24' 'FG25' 'FG26' 'FG27' 'FG28' 'FG29' 'FG30' 'FG31' 'FG32' 'FG33' 'FG34' 'FG35' 'FG36' 'FG37' 'FG38' 'FG39' 'FG40' 'FG41' 'FG42' 'FG43' 'FG44' 'FG45' 'FG46' 'FG47' 'FG48' 'FG49' 'FG50' 'FG51' 'FG52' 'FG53' 'FG54' 'FG55' 'FG56' 'FG57' 'FG58' 'FG59' 'FG60' 'FG61' 'FG62' 'FG63' 'FG64' 'OFG1' 'OFG2' 'OFG3' 'OFG4' 'OFG5' 'OFG6' 'OFG7' 'OFG8' 'OFG9' 'OFG10' 'OFG11' 'OFG12' 'OFG13' 'OFG14' 'OFG15' 'OFG16' 'OFG17' 'OFG18' 'OFG19' 'OFG20' >> ft_channelselection('FG*', hdr.label) ans = 'FG1' 'FG2' 'FG3' 'FG4' 'FG5' 'FG6' 'FG7' 'FG8' 'FG9' 'FG10' 'FG11' 'FG12' 'FG13' 'FG14' 'FG15' 'FG16' 'FG17' 'FG18' 'FG19' 'FG20' 'FG21' 'FG22' 'FG23' 'FG24' 'FG25' 'FG26' 'FG27' 'FG28' 'FG29' 'FG30' 'FG31' 'FG32' 'FG33' 'FG34' 'FG35' 'FG36' 'FG37' 'FG38' 'FG39' 'FG40' 'FG41' 'FG42' 'FG43' 'FG44' 'FG45' 'FG46' 'FG47' 'FG48' 'FG49' 'FG50' 'FG51' 'FG52' 'FG53' 'FG54' 'FG55' 'FG56' 'FG57' 'FG58' 'FG59' 'FG60' 'FG61' 'FG62' 'FG63' 'FG64'


Robert Oostenveld - 2017-01-17 11:29:43 +0100

closed multiple bugs that were resolved some time ago