Back to the main page.

Bug 2198 - throw an explicit warning by default when cfg options are not used?

Status NEW
Reported 2013-06-13 10:47:00 +0200
Modified 2013-06-13 13:04:30 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P3 normal
Assigned to:
URL:
Tags:
Depends on:
Blocks:
See also:

Roemer van der Meij - 2013-06-13 10:47:56 +0200

Just throwing it out there, refreshed after our discussion of cfg-monitoring yesterday. Can we think of an example where it is desired to add cfg options which are not used? Maybe some functions in the ft_XXXstatistics tree? Would there be options that are not read, but used later on? If we can't think of an example, wouldn't it be desired to explicitly mention this after whatever function is completed? Or maybe even go as extreme as throwing an error? Which would be more difficult, as usually only after all computation the appropriate cfg options are touched.


Robert Oostenveld - 2013-06-13 11:08:27 +0200

Sometimes options are not used by the initially called function, but by a low-level function. This makes it difficult (sometimes impossible) to figure out at the start of a function which options will be unused. Some spike functions make use of ft_checkconfig(cfg, 'allowed', {'method', 'channel', 'spikechannel', ...}) We could do that more often, i.e. in all functions that do pass the cfg into another function. Please have a look in ft_checkconfig. The second option is to start using the config object, but as it stands, it is not 100% struct-compatible.


Roemer van der Meij - 2013-06-13 11:21:07 +0200

It might not be a bad idea to explicitly allow only certain config options, in all functions. Just like with how keyvalcheck works in lower level functions. It will take some effort, and probably mistakes, to get such a list right for each function (especially the nested function calls). There are still 2 cfg object bugs that are assigned to, bug 1614 and bug 1762. The first bug still is not solved, I couldn't get a grasp on the cause of the error (for me, the recursion was partly to blame). The second bug still has an open question related to the nargout.


Jörn M. Horschig - 2013-06-13 11:29:24 +0200

I think I suggested this also a while ago, when I realized that some ft_sourceXXX function does not take the analog cfg options to the ft_freqXXX functions and wondered why. Instead of implementing this, Robert and me went over and adjusted names of the cfg fields. I can also strongly see the benefit of this, but as Robert said it is sometimes tricky, for example trialfuns or other user functions might be written such that they require some cfg-option being passed that is not FieldTrip standard. Thus, I would not throw an error but rather throw a neat warning saying that cfg.whatever might not be used. In any case, this needs to be carefully done for all functions, i.e. would be lots of work, because we do not want users to get scared by some warning that is not justified (and we do not want to get tons of mails on the mailingslist asking about such things either).


Robert Oostenveld - 2013-06-13 13:04:30 +0200

(In reply to comment #3) We might want to add warnings (which ft_checkconfig-allowed will issue for us) for some of the simple functions that do not call other functions. I think that in general it is a good coding style, i.e. be explicit about what works.