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.