Back to the main page.

Bug 1239 - Highlightchannel issue with topoplot

Reported 2011-12-21 19:36:00 +0100
Modified 2013-06-05 12:16:00 +0200
Product: FieldTrip
Component: plotting
Version: unspecified
Hardware: All
Operating System: All
Importance: P3 normal
Assigned to: Roemer van der Meij
Depends on:
See also:

Rodolphe - 2011-12-21 19:36:54 +0100

Recently, a bug has appeared with Topoplot functions and the highlightchannel option. When cfg.highlightchannel is empty, the Topoplot highlight all channels when it shouldnt highlight anyone. Tested with the very last version (-20111220), problem still occurs. Thanks

Boris Reuderink - 2011-12-22 14:09:01 +0100

See revision 5074, ft_topoplotTFR.m line 283. Probably the "emptymeaningful" boolean in "ft_getopt" should be set to true.

Jan-Mathijs Schoffelen - 2012-01-03 11:28:22 +0100

do we have some code to reproduce this problem?

Jan-Mathijs Schoffelen - 2012-01-03 12:17:05 +0100

OK, issue reproduced with cfg.highlight = 'on'; cfg.highlightchannel = {}; I wonder why somebody would like to do this to begin with, but OK. I made a fix (thanks to Boris) by adding the emptymeaningful flag in the respective line and battling a bit with the conditional statements that do some iffy-things with the highlight related stuff around line 310. The fix is implemented in revision 5092. I would consider restructuring this code such that the cfg.highlight option is removed, and that a (default) cfg.highlightchannel actually means that cfg.highlight = 'off'; I gladly pass this on to Roemer, who made the initial implementation ;-). Perhaps to be discussed first.

Roemer van der Meij - 2012-01-03 14:24:37 +0100

Hmmmm, I thought I made a rather elaborate set of if loops that would detect it, but I may have skipped this one (as highlighting channels that are made of thin air should not be allowed). We did discuss in the last FT meeting on how to handle the yes/no settings, specifically how to deal with options that have a 'no', or a selection of channels/options :). If I remember correctly, we agreed to remove such cfg options? Because it made a simpler, stricter and more elegant set of if-loops possible, by allowing us to use istrue/isfalse to default such things (and use logical flags in the if-loops)

Roemer van der Meij - 2012-02-01 14:22:36 +0100

Bug is fixed. Closeable after JM agrees with the last sentence. On the restructuring: We discussed the following in a meeting a while ago: 1) options that can be on/off/yes/no/true/false, should be only so and 2) these options should be passed through istrue, and 3) the if-statement should be a boolean, i.e. the output of istrue For topoplot, the cfg.highlight used to be the option that also immediately defined channels, or was set to 'off'. For compatibility I suggest we don't change it yet to fit the above, the current highlight stuff is about 1,5 years old, maybe we should support it for a little longer?

Roemer van der Meij - 2013-06-05 12:16:00 +0200

Closing time