Back to the main page.
Bug 2162 - avoid eval and feval in main fieldtrip functions
| Status | CLOSED FIXED |
| Reported | 2013-05-13 09:41:00 +0200 |
| Modified | 2014-03-06 15:38:22 +0100 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Mac OS |
| Importance: | P3 normal |
| Assigned to: | Jan-Mathijs Schoffelen |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Robert Oostenveld - 2013-05-13 09:41:17 +0200
making a deployed/compiled fieldtrip version is much easier if there are no unknown dependencies. If funname = sprintf('something_%s'. cfg.method) feval(funname) is done. the compiler does not know what function dependencies (e.g. from private) to include. Therefore it is to be preferred not to use eval/feval. There are exceptions, where it does make more sense (e.g. trialfun, statfun). When I search for possible occurences of the problem, I see mac001> grep eval\( *.m ft_compile_mex.m: eval(cmd); this is OK use ft_databrowser.m: feval(funhandle, funcfg, seldata); ft_databrowser.m: eval([code{icomm} ';']); this is OK use ft_definetrial.m: trl = feval(cfg.trialfun, cfg); ft_definetrial.m: [trl, event] = feval(cfg.trialfun, cfg); this is OK use ft_freqanalysis.m: [freq] = feval(sprintf('ft_freqanalysis_%s',lower(cfg.method)), cfg, data); TO BE FIXED, freqanalysis it could be hard-coded, like in ft_megplanar. Looking at the code in more detail, it only pertains to the old implementtaions. Should those not be deleted completely? They have been deprecated for some time. -> TODO discuss with Roemer ft_megplanar.m: %planarmontage = eval([fun '(cfg, data.grad)']); ft_megplanar.m: planarmontage = eval([fun '(cfg, data.grad)']); This is the perfect example ft_prepare_layout.m: eval(['img = ',tmp.name,';']); I suspect that this can be avoided. ft_rejectartifact.m: cfg = feval(sprintf('artifact_%s', cfg.artfctdef.type{type}), cfg, data); ft_rejectartifact.m: cfg = feval(sprintf('artifact_%s', cfg.artfctdef.type{type}), cfg); I think this is old-style calling syntax that should be reconsidered anyway. Similar to ft_freqanalysis. ft_sensorrealign.m: elec_realigned = ft_transform_sens(feval(cfg.warp, norm.m), elec_original); This can probably be replaced by ft_warp_apply or so. ft_sourcedescriptives.m: if isfield(source, 'avg' ) && isfield(source.avg , 'pow'), source.avg .pow = feval(cfg.transform, source.avg .pow); end ft_sourcedescriptives.m: if isfield(source, 'avgA' ) && isfield(source.avgA , 'pow'), source.avgA.pow = feval(cfg.transform, source.avgA.pow); end ft_sourcedescriptives.m: if isfield(source, 'avgB' ) && isfield(source.avgB , 'pow'), source.avgB.pow = feval(cfg.transform, source.avgB.pow); end ft_sourcedescriptives.m: if isfield(source, 'trial' ) && isfield(source.trial , 'pow'), for i=1:length(source.trial ), source.trial (i).pow = feval(cfg.transform, source.trial (i).pow); end; end ft_sourcedescriptives.m: if isfield(source, 'trialA') && isfield(source.trialA, 'pow'), for i=1:length(source.trialA), source.trialA(i).pow = feval(cfg.transform, source.trialA(i).pow); end; end ft_sourcedescriptives.m: if isfield(source, 'trialB') && isfield(source.trialB, 'pow'), for i=1:length(source.trialB), source.trialB(i).pow = feval(cfg.transform, source.trialB(i).pow); end; end ft_sourcedescriptives.m: if isfield(source, 'avg' ) && isfield(source.avg , 'noise'), source.avg .noise = feval(cfg.transform, source.avg .noise); end ft_sourcedescriptives.m: if isfield(source, 'avgA' ) && isfield(source.avgA , 'noise'), source.avgA.noise = feval(cfg.transform, source.avgA.noise); end ft_sourcedescriptives.m: if isfield(source, 'avgB' ) && isfield(source.avgB , 'noise'), source.avgB.noise = feval(cfg.transform, source.avgB.noise); end ft_sourcedescriptives.m: if isfield(source, 'trial' ) && isfield(source.trial , 'noise'), for i=1:length(source.trial ), source.trial (i).noise = feval(cfg.transform, source.trial (i).noise); end; end ft_sourcedescriptives.m: if isfield(source, 'trialA') && isfield(source.trialA, 'noise'), for i=1:length(source.trialA), source.trialA(i).noise = feval(cfg.transform, source.trialA(i).noise); end; end ft_sourcedescriptives.m: if isfield(source, 'trialB') && isfield(source.trialB, 'noise'), for i=1:length(source.trialB), source.trialB(i).noise = feval(cfg.transform, source.trialB(i).noise); end; end this is not to be touched. ft_wizard.m: assignin('base', wizard_var(wizard_i).name, eval(wizard_var(wizard_i).name)); ft_wizard.m: varargout{1}.(wizard_var(wizard_i).name) = eval(wizard_var(wizard_i).name); this is not to be touched.
Robert Oostenveld - 2013-05-13 09:43:08 +0200
(In reply to comment #0) so in short 1) ft_prepare_layout and ft_sensorrealign should be checked in more detail. 2) ft_freqanalysis and ft_rejectartifact seem to use it in old backward-compatibility sections of the code. There we have to discuss whether those sections can be removed altogether.