[FIX] save onsets.mat directly in subject stats folder (#1187)

* save onsets.mat directly in FFX dir * update changelog * rename file
cpp-lln-lab · Jan 25, 2024 · 55e975d · 55e975d
1 parent a4a19e1
commit 55e975d
Show file tree

Hide file tree

Showing 20 changed files with 68 additions and 85 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+* [FIX] save onsets.mat directly in subject stats folder #1187 by @Remi-Gau
 * [FIX] do not compute subject level contrast when running dataset level #1102 by @Remi-Gau
 * [FIX] copy `RepetitionTime` in sidecar JSON after running smoothing in #1099 by @Remi-Gau
 * [FIX] rename results files (csv, tsv, png, nii) of each contrasts #1104 by @Remi-Gau

diff --git a/src/stats/subject_level/convertOnsetTsvToMat.m b/src/stats/subject_level/convertOnsetTsvToMat.m
@@ -1,4 +1,4 @@
-function fullpathOnsetFilename = convertOnsetTsvToMat(opt, tsvFile, runDuration)
+function fullpathOnsetFilename = convertOnsetTsvToMat(opt, tsvFile, runDuration, outputDir)
   %
   % Converts an events.tsv file to an onset file suitable for SPM subject level analysis.
   %
@@ -17,6 +17,9 @@
   %                     Events occurring later than this will be excluded.
   % :type  runDuration: numeric
   %
+  % :param outputDir: Path where to save ``onset.mat``. Optional.
+  % :type  outputDir: path
+  %
   % Use a BIDS stats model specified in a JSON file to:
   %
   % - loads events.tsv and apply the ``Node.Transformations`` to its content
@@ -59,6 +62,10 @@
     runDuration = nan;
   end
 
+  if nargin < 4
+    outputDir = '';
+  end
+
   REQUIRED_COLUMNS = {'onset', 'duration'};
 
   [pth, file, ext] = spm_fileparts(tsvFile);
@@ -196,6 +203,9 @@
   bf.suffix = 'onsets';
   bf.extension = '.mat';
 
+  if ~strcmp(outputDir, '')
+    pth = outputDir;
+  end
   fullpathOnsetFilename = fullfile(pth, bf.filename);
 
   names = condToModel.names; %#ok<*NASGU>

diff --git a/src/stats/subject_level/createAndReturnOnsetFile.m b/src/stats/subject_level/createAndReturnOnsetFile.m
@@ -39,18 +39,7 @@
   msg = sprintf('\n  Reading the tsv file : %s \n', bids.internal.format_path(tsvFile));
   logger('INFO', msg, 'options', opt, 'filename', mfilename());
 
-  onsetFilename = convertOnsetTsvToMat(opt, tsvFile, runDuration);
-
-  % move file into the FFX directory
-  [~, filename, ext] = spm_fileparts(onsetFilename);
-
-  % reset task query to original value
-  % in case we are merging several tasks in one GLM
-  opt.query.task = opt.taskName;
-
   ffxDir = getFFXdir(subLabel, opt);
-  movefile(onsetFilename, ffxDir);
-
-  onsetFilename = fullfile(ffxDir, [filename ext]);
+  onsetFilename = convertOnsetTsvToMat(opt, tsvFile, runDuration, ffxDir);
 
 end
diff --git a/tests/tests_batches/preproc/test_setBatchGenerateT1map.m b/tests/tests_batches/preproc/test_setBatchGenerateT1map.m
@@ -39,9 +39,7 @@ function test_setBatchGenerateT1map_basic()
 
 function test_setBatchGenerateT1map_warning()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   subLabel = '^01';
 

diff --git a/tests/tests_batches/stats/test_setBatchSubjectLevelResults.m b/tests/tests_batches/stats/test_setBatchSubjectLevelResults.m
@@ -52,9 +52,7 @@ function test_setBatchSubjectLevelResults_basic()
 
 function test_setBatchSubjectLevelResults_missing_contrast_name()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   [subLabel, opt, result] = setUp('vismotion');
 
@@ -71,9 +69,7 @@ function test_setBatchSubjectLevelResults_missing_contrast_name()
 
 function test_setBatchSubjectLevelResults_error_no_matching_contrast()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   contrast_name = 'NotAContrast';
   [subLabel, opt, result] = setUp('vismotion', contrast_name);

diff --git a/tests/tests_bids/test_getAnatFilename.m b/tests/tests_bids/test_getAnatFilename.m
@@ -31,9 +31,7 @@ function test_getAnatFilename_return_several()
   assertEqual(numel(anatImage), 3);
   warning('ON');
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   assertWarning(@()getAnatFilename(BIDS, opt, subLabel, nbImgToReturn), ...
                 'getAnatFilename:severalAnatFile');

diff --git a/tests/tests_bids_model/test_bidsModel.m b/tests/tests_bids_model/test_bidsModel.m
@@ -162,9 +162,7 @@ function test_getVariablesToConvolve_warning()
   opt = setOptions('vislocalizer');
   bm = BidsModel('file', opt.model.file);
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   assertWarning(@()bm.getVariablesToConvolve('Name', 'dataset_level'), ...
                 'BidsModel:noVariablesToConvolve');
 

diff --git a/tests/tests_defaults/test_checkOptions.m b/tests/tests_defaults/test_checkOptions.m
@@ -56,9 +56,7 @@ function test_checkOptions_do_not_overwrite()
 
 function test_checkOptions_error_task()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   opt.taskName = '';
   opt.verbosity = 1;

diff --git a/tests/tests_infra/test_checkToolbox.m b/tests/tests_infra/test_checkToolbox.m
@@ -16,9 +16,7 @@ function test_checkToolbox_mp2rage()
 
   assertEqual(status, isdir(fullfile(spm('dir'), 'toolbox', 'mp2rage')));
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   if ~isdir(fullfile(spm('dir'), 'toolbox', 'mp2rage'))
     assertWarning(@()checkToolbox('mp2rage', 'verbose', true), ...

diff --git a/tests/tests_slow/tests_batches/stats/test_setBatchFactorialDesign.m b/tests/tests_slow/tests_batches/stats/test_setBatchFactorialDesign.m
@@ -117,9 +117,7 @@ function test_setBatchFactorialDesign_wrong_model_design_matrix()
 
   matlabbatch = {};
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   assertWarning(@()setBatchFactorialDesign(matlabbatch, opt, datasetNode.Name), ...
                 'setBatchFactorialDesign:notImplemented');
 

diff --git a/tests/tests_slow/tests_workflows/stats/test_bidsResults.m b/tests/tests_slow/tests_workflows/stats/test_bidsResults.m
@@ -106,9 +106,7 @@ function test_bidsResults_no_results()
 
   markTestAs('slow');
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   opt = setOptions('vismotion', '', 'pipelineType', 'stats');
 
@@ -183,9 +181,7 @@ function test_bidsResults_too_many_backgrounds()
 
   opt = rmResultsFromModel(opt);
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   opt.verbosity = 1;
   assertWarning(@()bidsResults(opt), 'bidsResults:tooManyMontageBackground');
 
@@ -258,9 +254,7 @@ function test_bidsResults_no_background_for_montage()
 
   opt = rmResultsFromModel(opt);
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   opt.verbosity = 1;
   assertWarning(@()bidsResults(opt), 'checkMaskOrUnderlay:missingMaskOrUnderlay');
 

diff --git a/tests/tests_stats/subject_level/test_convertOnsetTsvToMat.m b/tests/tests_stats/subject_level/test_convertOnsetTsvToMat.m
@@ -130,9 +130,7 @@ function test_convertOnsetTsvToMat_globbing_conditions()
 
   opt.verbosity = 1;
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   assertWarning(@() convertOnsetTsvToMat(opt, tsvFile), ...
                 'convertOnsetTsvToMat:variableNotFound');
 
@@ -182,6 +180,26 @@ function test_convertOnsetTsvToMat_basic()
 
 end
 
+function test_convertOnsetTsvToMat_output_dir()
+
+  % GIVEN
+  opt = setOptions('vismotion');
+  [opt, tmpDir] = setUp(opt);
+  tsvFile = fullfile(tmpDir, 'sub-01_task-vismotion_events.tsv');
+
+  % WHEN
+  runDuration = nan;
+  outputDir = fullfile(tmpDir, 'foo');
+  spm_mkdir(outputDir);
+  fullpathOnsetFilename = convertOnsetTsvToMat(opt, tsvFile, runDuration, outputDir);
+
+  % THEN
+  assertEqual(fullfile(outputDir, 'sub-01_task-vismotion_onsets.mat'), ...
+              fullpathOnsetFilename);
+  assertEqual(exist(fullpathOnsetFilename, 'file'), 2);
+
+end
+
 function test_convertOnsetTsvToMat_input_from_non_trial_type_column
 
   % GIVEN
@@ -261,9 +279,7 @@ function test_convertOnsetTsvToMat_missing_trial_type()
 
   opt.verbosity = 1;
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   assertWarning(@() convertOnsetTsvToMat(opt, tsvFile), ...
                 'convertOnsetTsvToMat:trialTypeNotFound');

diff --git a/tests/tests_stats/subject_level/test_getEventSpecificationRoiGlm.m b/tests/tests_stats/subject_level/test_getEventSpecificationRoiGlm.m
@@ -45,9 +45,7 @@ function test_getEventSpecificationRoiGlm_basic()
 
 function test_getEventSpecificationRoiGlm_warning_complex_contrasts()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   % GIVEN
   [modelFile, spmFile] = setUp();

diff --git a/tests/tests_stats/unit_tests/test_getInclusiveMask.m b/tests/tests_stats/unit_tests/test_getInclusiveMask.m
@@ -27,9 +27,7 @@ function test_getInclusiveMask_too_many()
 
   BIDS = getLayout(opt);
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   assertWarning(@()getInclusiveMask(opt, nodeName, BIDS, subLabel), ...
                 'getInclusiveMask:tooManyMasks');
@@ -58,9 +56,7 @@ function test_getInclusiveMask_basic()
 
 function test_getInclusiveMask_no_image()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   subLabel = '01';
   nodeName = 'run_level';

diff --git a/tests/tests_stats/utils/test_getRegressorIdx.m b/tests/tests_stats/utils/test_getRegressorIdx.m
@@ -54,9 +54,7 @@ function test_getRegressorIdx_basic()
   assertEqual(status, true);
 
   %% missing
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   assertWarning(@()getRegressorIdx('foo', SPM), 'getRegressorIdx:missingRegressor');
 
   sts = warning('QUERY', 'getRegressorIdx:missingRegressor');

diff --git a/tests/tests_unit/test_copyGraphWindownOutput.m b/tests/tests_unit/test_copyGraphWindownOutput.m
@@ -33,9 +33,7 @@ function test_copyGraphWindownOutput_basic()
 
 function test_copyGraphWindownOutput_warning()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   [opt, subLabel, action, PWD] = setUp();
   opt.verbosity = 1;

diff --git a/tests/tests_unit/test_labelActivations.m b/tests/tests_unit/test_labelActivations.m
@@ -79,9 +79,7 @@ function test_labelActivations_aal()
 
 function test_labelActivations_bug_662()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
 
   csvFile = fullfile(getTestDataDir(), 'tsv_files', 'bug662_results_table.csv');
 

diff --git a/tests/tests_unit/test_removeDummies.m b/tests/tests_unit/test_removeDummies.m
@@ -32,9 +32,7 @@ function test_removeDummies_not_force()
   inputFile = setUp();
   metadata.NumberOfVolumesDiscardedByUser = 10;
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   assertWarning(@()removeDummies(inputFile, 4, metadata), ...
                 'removeDummies:dummiesAlreadyRemoved');
 

diff --git a/tests/tests_unit/test_warnings.m b/tests/tests_unit/test_warnings.m
@@ -29,9 +29,7 @@ function test_noSPMmat()
   % THEN
   assertEqual(status, true);
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   opt.verbosity = 1;
   assertWarning(@()noSPMmat(opt, subLabel, spmMatFile), 'noSPMmat:noSpecifiedModel');
 
@@ -47,19 +45,15 @@ function test_noRoiFound()
   % THEN
   assertEqual(status, true);
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   opt.verbosity = 1;
   assertWarning(@()noRoiFound(opt, roiList), 'noRoiFound:noRoiFile');
 
 end
 
 function test_notImplemented()
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   opt.verbosity = 1;
   assertWarning(@()notImplemented('foo', '', opt), 'foo:notImplemented');
 
@@ -81,9 +75,7 @@ function test_isTtest()
   % THEN
   assertEqual(status, false);
 
-  if bids.internal.is_octave()
-    moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown');
-  end
+  skipIfOctave('mixed-string-concat warning thrown');
   assertWarning(@()isTtest(tmp), 'isTtest:notImplemented');
 
 end
diff --git a/tests/utils/skipIfOctave.m b/tests/utils/skipIfOctave.m
@@ -0,0 +1,11 @@
+function skipIfOctave(msg)
+  %
+  % skip test if running on octave
+  %
+
+  % (C) Copyright 2024 bidspm developers
+  if bids.internal.is_octave()
+    moxunit_throw_test_skipped_exception(['Octave:', msg]);
+  end
+
+end