Back to the main page.
Bug 1239 - Highlightchannel issue with topoplot
| Status | CLOSED FIXED |
| 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 |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| 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?