Back to the main page.

Bug 1502 - forbid fields in cfg that are not used

Status CLOSED FIXED
Reported 2012-06-03 10:38:00 +0200
Modified 2012-10-24 10:43:42 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P3 enhancement
Assigned to: Jan-Mathijs Schoffelen
URL:
Tags:
Depends on:
Blocks:
See also:

Martin Vinck - 2012-06-03 10:38:01 +0200

It sometimes occurs that a user mistypes a field name and that a default is used without the user knowing it. Perhaps in checkconfig we should have a sentence like cfg = ft_checkconfig(cfg,keyname,{'field1', 'field2', 'field3'}) where any cfg.field that is not in {'field1', 'field2', 'field3'} is rejected with an error. there's already allowed so perhaps you have a suggestion for keyname


Robert Oostenveld - 2012-06-03 12:22:59 +0200

there is already the option 'forbidden', see http://code.google.com/p/fieldtrip/source/browse/trunk/utilities/ft_checkconfig.m#37 Your suggestion is more like "forbid all fields except those that are explicitly allowed'. I suggest we add cfg = ft_checkconfig(cfg,'allowed',{'field1', 'field2', 'field3'}) to forbid all fields except the three listed.


Robert Oostenveld - 2012-06-03 12:43:50 +0200

I have implemented the 'allowed' option and a test script. Please have a look. mbp> svn commit utilities/ft_checkconfig.m test/test_bug1502.m Adding test/test_bug1502.m Sending utilities/ft_checkconfig.m Transmitting file data .. Committed revision 5840. Potential problems might be expected because of merging the global ft_default into the present cfg. Also non-trivial is the order of processing the various allowed/renamed/required options, although I think at the moment it is ok.


Martin Vinck - 2012-06-03 13:22:10 +0200

(In reply to comment #2) The implemented change works indeed. Indeed, the order of calling ft_checkconfig needs to be carefully checked then.


Robert Oostenveld - 2012-08-23 10:33:51 +0200

closed multiple bugs that have been resolved for some time


Jan-Mathijs Schoffelen - 2012-10-15 17:35:33 +0200

with the new ft_version handling this bug appears on the dashboard again. It goes wrong in line 202 ft_checkconfig, where the new fields 'trackcallinfo','trackdatainfo', and 'trackparaminfo' don't survive the check of being allowed. Suggested fix is to allow those fields starting from line 198. @Robert: if not agreed then this change should be undone.


Jan-Mathijs Schoffelen - 2012-10-15 17:37:30 +0200

bash-3.2$ pwd /home/language/jansch bash-3.2$ cd matlab/fieldtrip/utilities/ bash-3.2$ svn diff ft_checkconfig.m Index: ft_checkconfig.m =================================================================== --- ft_checkconfig.m (revision 6755) +++ ft_checkconfig.m (working copy) @@ -195,6 +195,9 @@ 'checkconfig' 'checksize' 'showcallinfo' + 'trackcallinfo' + 'trackdatainfo' + 'trackparaminfo' }); fieldsused = fieldnames(cfg); [c, i] = setdiff(fieldsused, allowed); bash-3.2$ svn commit ft_checkconfig.m -m 'bugfix - explicitly allow fields that relate to provenance tracking' Sending ft_checkconfig.m Transmitting file data . Committed revision 6759. bash-3.2$