Back to the main page.

Bug 1114 - make a dependency table for compat and ensure that all dependencies are appropriate

Status CLOSED FIXED
Reported 2011-11-04 08:32:00 +0100
Modified 2014-02-24 10:56:21 +0100
Product: FieldTrip
Component: core
Version: unspecified
Hardware: All
Operating System: All
Importance: P3 enhancement
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on: 12342083
Blocks:
See also: http://bugzilla.fcdonders.nl/show_bug.cgi?id=1234

Robert Oostenveld - 2011-11-04 08:32:33 +0100

after making the table, we need to identify the problematic cases and discuss them - no fieldtrip functions should depend on a compat function - all functions should only depend on functions at the same level or below


Robert Oostenveld - 2011-11-04 08:33:07 +0100

Created attachment 180 helper function to simplify the task


Boris Reuderink - 2011-11-15 10:31:25 +0100

What is the goal of tracking the dependencies? I can imagine: a) trigger the correct test scripts on modification the functions being tested, AND their dependencies, b) identify and clean up legacy dependencies c) documentation purposes. Did I miss anything?


Boris Reuderink - 2012-01-04 10:33:53 +0100

Started WIP code as ft_dependencies.m.


Boris Reuderink - 2012-01-04 15:12:04 +0100

I compared the current mex files with the files listed in ft_compile_mex, and found some inconsistencies already: src/nanvar.c was not listed src/keyval.c does not exist The mexfiles were found using: $ find . -name "*.c" | xargs grep -l mexFunction | sort Maybe we should automatically update the list with mexfiles? The main question would be how and when these files should be updated...


Boris Reuderink - 2012-01-04 15:48:13 +0100

Hmm. I see that I have commented on the wrong bug. Comments 3 & 4 were meant for bug 1234.


Boris Reuderink - 2012-01-17 12:24:03 +0100

Added time estimate.


Robert Oostenveld - 2012-02-07 10:01:23 +0100

(In reply to comment #0) - no fieldtrip functions should depend on a compat function I created a test script that checks for undesired compat usage and that gives an error if compat-usage is detected. mbp> svn add test_bug1114.m A test_bug1114.m mbp> svn commit test_bug1114.m Adding test_bug1114.m Transmitting file data . Committed revision 5251.


Robert Oostenveld - 2012-02-07 10:04:36 +0100

what remains to be done is listed in the test script (on which I was working on the plane, so I could not commit immediately) % TODO % 1. crimic should be explained how to use compat % 2. fixed a spike function (channelselection), should be committed % 3. fieldtrip/compat/openmeeg.m should be removed % 4. fieldtrip/ft_headmodelplot.m should be removed % 5. compat/ft_prepare_bemmodel.m and ft_prepare_bemmodel.m should be merged, the compat one should then be removed I have already done the 2nd mbp> svn commit ft_spiketriggeredspectrum_tfr.m Sending ft_spiketriggeredspectrum_tfr.m Transmitting file data . Committed revision 5252. I will do the others when I am back together with Cristiano. PS @Boris: I'll assign this one to me


Robert Oostenveld - 2012-02-10 16:47:19 +0100

processing ... '/Users/robert/matlab/fieldtrip/utilities/compat/progress.m' 'beamformer_sam.m' Warning: some of the FT functions depend on compat functions


Robert Oostenveld - 2012-02-10 16:50:18 +0100

I have extended the test function so that it also tests the modules and private mbp> svn commit test_bug1114.m Sending test_bug1114.m Transmitting file data . Committed revision 5276.


Cristiano Micheli - 2012-02-15 16:55:59 +0100

(In reply to comment #10) % TODO % 1. crimic should be explained how to use compat got it % 3. fieldtrip/compat/openmeeg.m should be removed It's gone! I don't see it neither in my local copy, nor in the /home/common, somebody already did it? % 4. fieldtrip/ft_headmodelplot.m should be removed done % 5. compat/ft_prepare_bemmodel.m and ft_prepare_bemmodel.m should be merged, the compat one should then be removed done What's next? Cheers, Cristiano


Roemer van der Meij - 2012-04-12 13:38:01 +0200

The test script, test_bug1114 returned with an error in the test-batching. The script checks for dependencies on compat/functions and throws an error if any dependencies on compat are found in non-compat-functions. It detected 3 dependencies on compat functions. 2 of those were actual dependencies, on filetype.m instead of ft_filetype.m and were fixed (nice work on automatically checking this!). However, the 3 dependency is a bit weird. It is listed as a dependency on compat/fieldtripdefs.m, which is called by trunk/fieldtripdefs.m. If I do a which fieldtripdefs -all, it returns trunk/fieldtripdefs as a 'shadow'. Is this 'shadow' here on purpose? If so, I will add an exception to the test script, ignoring the shadow. If not, what is a shadow and how do we get rid of it? :)


Robert Oostenveld - 2012-04-12 13:44:42 +0200

(In reply to comment #12) in general functions should only be present once (either in main, a module or in a compat folder). Note that this is a special case, because fieldtripdefs was called automatically by all FT functions. In this case I suggest to get rid of both functions! People should not be using this any more.


Roemer van der Meij - 2012-04-12 14:16:15 +0200

(In reply to comment #13) Both have now been removed.


Robert Oostenveld - 2013-03-16 15:35:41 +0100

I have created a new function ft_dependencycheck and added private/mydepfun. Using these, I detected two dependencies on nnet/dist, and one dependency on simulink/float. Both were easy to remove. Remaining dependencies on Mathworks toolboxes are The following functions depend on the Mathworks "images" toolbox: ft_sourceplot ft_volumesegment ft_read_mri The following functions depend on the Mathworks "optim" toolbox: warp_optim dipole_fit The following functions depend on the Mathworks "signal" toolbox: ft_mvaranalysis ft_resampledata ft_preproc_bandpassfilter ft_preproc_bandstopfilter ft_preproc_denoise ft_preproc_highpassfilter ft_preproc_hilbert ft_preproc_lowpassfilter ft_preproc_medianfilter ft_preproc_resample ft_specest_hilbert ft_specest_mtmconvol ft_specest_mtmfft ft_spiketriggeredspectrum_convol ft_spiketriggeredspectrum_fft The following functions depend on the Mathworks "stats" toolbox: ft_connectivitysimulation ft_headmovement ft_qualitycheck ft_regressconfound ft_sourcedescriptives ft_statistics_stats ft_stratify ft_datatype_spike ft_statfun_depsamplesF ft_statfun_indepsamplesF ft_statfun_indepsamplesZcoh ft_spike_isi ft_spike_plot_isi ft_spike_plot_isireturn ft_spike_plot_jpsth ft_spike_plot_raster ft_spike_waveform ft_spike_xcorr ft_spikesorting ft_spiketriggeredspectrum_convol


Robert Oostenveld - 2013-03-16 15:49:26 +0100

I have updated the documentation on http://fieldtrip.fcdonders.nl/faq/matlab_requirements


Robert Oostenveld - 2013-06-28 12:13:00 +0200

I am about to finally merge and close this bug, but had some issues due to it being branched quite some time ago, resulting in too many changes. For reference: here are the relevant ones. Mar 16, 2013 oostenveld enhancement - removed dependency on simulink float function, use sing… … 31c3fc4 oostenveld enhancement - avoid dependency on nnet toolbox using replacement "dist" efb9ed2 oostenveld enhancement - avoid dependency on nnet toolbox by using replacement f… … b685f55 oostenveld enhancement - implemented function to check dependencies, see also ht… … aac62b3 oostenveld Merge branch 'master' into bug1114 daf3414 Jun 28, 2013 oostenveld enhancement - avoid dependency on nnet toolbox by using replacement f… … 965d6bd oostenveld Merge branch 'bug1114' of github.com:oostenveld/fieldtrip into bug1114 f26c996 mac001> git commit forward/ft_prepare_vol_sens.m [bug1114 ff08a5f] enhancement - final cleanups for http://bugzilla.fcdonders.nl/show_bug.cgi?id=1114, about to close the bug 1 file changed, 2 deletions(-)


Robert Oostenveld - 2013-06-28 12:40:24 +0200

I merged the changes related to bug 1114, 2209, 1961 and 1792 into the svn repository


Robert Oostenveld - 2013-06-28 12:41:23 +0200

(In reply to comment #18) see http://code.google.com/p/fieldtrip/source/detail?r=8285


Robert Oostenveld - 2013-09-18 13:19:10 +0200

closed multiple bugs that have been resolved


Jörn M. Horschig - 2013-09-25 10:44:44 +0200

I just saw that test script fails, because JM added a compat version for ft_prepare_atlas.m for the time being


Robert Oostenveld - 2013-09-25 11:03:35 +0200

(In reply to comment #21) and mac001> grep ft_prepare_atlas *.m Contents.m:% ft_prepare_atlas ft_volumelookup.m: atlas = ft_prepare_atlas(cfg); ft_volumelookup.m: atlas = ft_prepare_atlas(cfg.atlas); So the compat function is being called.


Robert Oostenveld - 2013-09-25 11:05:07 +0200

(In reply to comment #22) I am reopening this, as it depends on bug 2083 to be resolved.


Robert Oostenveld - 2014-02-24 09:12:45 +0100

(In reply to Robert Oostenveld from comment #23) bug 2083 has been resolved, so this one can be closed as well.


Robert Oostenveld - 2014-02-24 10:56:21 +0100

I closed several bugs at once that all have been resolved for some time. If you disagree, please reopen.