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