Back to the main page.

Bug 3135 - ft_channelselection selects channels it shouldn't because wild card is added

Status CLOSED FIXED
Reported 2016-06-03 20:06:00 +0200
Modified 2019-08-10 12:33:18 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P5 critical
Assigned to: Jörn M. Horschig
URL:
Tags:
Depends on: 2028
Blocks:
See also:

Roemer van der Meij - 2016-06-03 20:06:55 +0200

>> ft_channelselection({'Cz','C3'},{'Cz','FCz','FC3'}) ans = 'Cz' 'FCz' 'FC3' Only Cz should have been selected. However, FCz and FC3 are incorrectly selected as well. On line 182 a wild card is added onto Cz and C3, causing a match with FCz and FC3. There are many situations in which channel names are a part of other channel names, like most EEG caps? It seems to be in the code for at least a year, since: https://github.com/fieldtrip/fieldtrip/commit/4fcfa268538dcf198d1f7219cce4164a416c6592 I've set this to critical, as this can lead to very undesired behavior when averaging over channels, or any other procedure where it the inclusion of extra channels is not immediately obvious. CC Robert, JM, and Jorn (the above commit, from bug 2028)


Roemer van der Meij - 2016-06-03 20:08:14 +0200

Added CCs


Jörn M. Horschig - 2016-06-06 11:51:25 +0200

My sincere apologies. I put the $ at the end, because (according to how I understood the documentation) I thought it puts the correct constraint on when to remove or select what channels. It seems that putting an additional ^ in front resolves the issue. rexp = sprintf('%s%s%s', '^', regexptranslate('wildcard',channel{i}), '$'); lreg = ~cellfun(@isempty, regexp(datachannel, rexp)); This tells the regexp that the channel to be selected has to start (and to end) with what the user asks for. If the users puts wildcards into his selection, like *, then of course it can end (or start) with anything. Note that currently C* will also return FCz, as C* occurs within the string. With the above addition, this will not happen. A pity that the channelselection test script did not check for this, or any other test script caught this :(


Roemer van der Meij - 2016-06-06 19:19:14 +0200

Hmmm, I'm not confident enough with regular expressions to mess around with it. Can you check and implement the fix? And add it to test_ft_channelselection? Since it's so important to many functions, let's be sure it also works across matlab versions. I have a 2012 and 2015 currently installed, so can at least help with testing it if you put it in a github branch.


Jörn M. Horschig - 2016-06-07 17:37:24 +0200

I have no time the coming days to get back to this (I also need to get git working with my ft version, etc). From next week on I could do this though.


Jörn M. Horschig - 2016-06-10 17:05:48 +0200

https://github.com/fieldtrip/fieldtrip/pull/176/files


Robert Oostenveld - 2019-08-10 12:33:18 +0200

This closes a whole series of bugs that have been resolved (either FIXED/WONTFIX/INVALID) for quite some time. If you disagree, please file a new issue on https://github.com/fieldtrip/fieldtrip/issues.