From 5f5f4f9b9824e3208349725ac8802edc3749f148 Mon Sep 17 00:00:00 2001 From: Remi Gau Date: Mon, 29 Jul 2024 12:33:26 +0200 Subject: [PATCH] several fixes --- docs/source/API.rst | 16 +++++++++++----- src/batches/stats/setBatchGroupLevelContrasts.m | 10 ++-------- .../stats/setBatchSubjectLevelContrasts.m | 4 ++-- src/batches/stats/setBatchSubjectLevelGLMSpec.m | 2 +- src/bids/getAvailableGroups.m | 5 +++++ src/bids_model/getContrastsList.m | 2 +- .../getContrastsListForFactorialDesign.m | 2 +- src/stats/subject_level/specifyContrasts.m | 2 +- src/workflows/roi/bidsRoiBasedGLM.m | 6 +++--- .../preproc/test_setBatchGenerateT1map.m | 2 +- tests/tests_bids/test_fileFilterForBold.m | 14 +++++++------- .../tests_bids_model/test_getOptionsFromModel.m | 8 ++++---- tests/tests_bids_model/test_validateGroupBy.m | 6 +++--- .../tests_workflows/stats/test_bidsResults.m | 2 +- .../test_renameSegmentParameter.m | 2 +- .../tests_workflows/test_renameUnwarpParameter.m | 2 +- .../unit_tests/test_getInclusiveMask.m | 4 ++-- 17 files changed, 47 insertions(+), 42 deletions(-) diff --git a/docs/source/API.rst b/docs/source/API.rst index 62447b1a9..f03955971 100644 --- a/docs/source/API.rst +++ b/docs/source/API.rst @@ -93,8 +93,8 @@ stats .. autofunction:: src.batches.stats.setBatchContrasts .. autofunction:: src.batches.stats.setBatchEstimateModel .. autofunction:: src.batches.stats.setBatchFactorialDesign +.. autofunction:: src.batches.stats.setBatchFactorialDesignGlobalCalcAndNorm .. autofunction:: src.batches.stats.setBatchFactorialDesignImplicitMasking -.. autofunction:: src.batches.stats.setBatchFatorialDesignGlobalCalcAndNorm .. autofunction:: src.batches.stats.setBatchGoodnessOfFit .. autofunction:: src.batches.stats.setBatchGroupLevelContrasts .. autofunction:: src.batches.stats.setBatchGroupLevelResults @@ -173,6 +173,7 @@ bids .. autofunction:: src.bids.getAnatFilename .. autofunction:: src.bids.getAndCheckRepetitionTime .. autofunction:: src.bids.getAndCheckSliceOrder +.. autofunction:: src.bids.getAvailableGroups .. autofunction:: src.bids.getBoldFilename .. autofunction:: src.bids.getInfo .. autofunction:: src.bids.getMeanFuncFilename @@ -188,7 +189,6 @@ bids bids_model ========== .. autofunction:: src.bids_model.checkContrast -.. autofunction:: src.bids_model.checkGroupBy .. autofunction:: src.bids_model.createDefaultStatsModel .. autofunction:: src.bids_model.createModelFamilies .. autofunction:: src.bids_model.getContrastsFromParentNode @@ -198,6 +198,12 @@ bids_model .. autofunction:: src.bids_model.getDummyContrastsList .. autofunction:: src.bids_model.getInclusiveMask +bidspm +====== + +data +---- + cli === .. autofunction:: src.cli.baseInputParser @@ -220,6 +226,7 @@ cli constants ========= +.. autofunction:: src.constants.bidsAppsActions .. autofunction:: src.constants.lowLevelActions defaults @@ -262,7 +269,6 @@ messages .. autofunction:: src.messages.errorHandling .. autofunction:: src.messages.logger .. autofunction:: src.messages.noRoiFound -.. autofunction:: src.messages.noSPMmat .. autofunction:: src.messages.notImplemented .. autofunction:: src.messages.printAvailableContrasts .. autofunction:: src.messages.printBatchName @@ -339,7 +345,6 @@ subject_level .. autofunction:: src.stats.subject_level.getSessionForRegressorNb .. autofunction:: src.stats.subject_level.newContrast .. autofunction:: src.stats.subject_level.orderAndPadCounfoundMatFile -.. autofunction:: src.stats.subject_level.removeIntercept .. autofunction:: src.stats.subject_level.reorderCounfounds .. autofunction:: src.stats.subject_level.sanitizeConfounds .. autofunction:: src.stats.subject_level.saveRoiGlmSummaryTable @@ -354,10 +359,10 @@ group_level .. autofunction:: src.stats.group_level.computeCumulativeFwhm .. autofunction:: src.stats.group_level.findSubjectConImage .. autofunction:: src.stats.group_level.getRFXdir -.. autofunction:: src.stats.group_level.groupLevelGlmType utils ----- +.. autofunction:: src.stats.utils.checkSpmMat .. autofunction:: src.stats.utils.createGlmDirName .. autofunction:: src.stats.utils.designMatrixFigureName .. autofunction:: src.stats.utils.endsWithRunNumber @@ -366,6 +371,7 @@ utils .. autofunction:: src.stats.utils.getRegressorIdx .. autofunction:: src.stats.utils.labelActivations .. autofunction:: src.stats.utils.labelSpmSessWithBidsSesAndRun +.. autofunction:: src.stats.utils.removeIntercept .. autofunction:: src.stats.utils.returnContrastImageFile .. autofunction:: src.stats.utils.returnNumberScrubbedTimePoints .. autofunction:: src.stats.utils.validateContrasts diff --git a/src/batches/stats/setBatchGroupLevelContrasts.m b/src/batches/stats/setBatchGroupLevelContrasts.m index 7615b2225..a1179e6c8 100644 --- a/src/batches/stats/setBatchGroupLevelContrasts.m +++ b/src/batches/stats/setBatchGroupLevelContrasts.m @@ -122,14 +122,8 @@ case 'two_sample_t_test' - designMatrix = removeIntercept(designMatrix); - - if any(ismember(designMatrix, fieldnames(participants))) - % TODO will this ignore the contrasts define at other levels - % and not passed through the filter ? - edge = bm.get_edge('Destination', nodeName); - contrastsList = edge.Filter.contrast; - end + edge = bm.get_edge('Destination', nodeName); + contrastsList = edge.Filter.contrast; for j = 1:numel(contrastsList) diff --git a/src/batches/stats/setBatchSubjectLevelContrasts.m b/src/batches/stats/setBatchSubjectLevelContrasts.m index 54a2e8544..707ea9e19 100644 --- a/src/batches/stats/setBatchSubjectLevelContrasts.m +++ b/src/batches/stats/setBatchSubjectLevelContrasts.m @@ -27,10 +27,10 @@ printBatchName('subject level contrasts specification', opt); - spmMatFile = fullfile(getFFXdir(subLabel, opt), 'SPM.mat'); - if ~checkSpmMat(dir, opt) + if ~checkSpmMat(getFFXdir(subLabel, opt), opt) return end + spmMatFile = fullfile(getFFXdir(subLabel, opt), 'SPM.mat'); load(spmMatFile, 'SPM'); diff --git a/src/batches/stats/setBatchSubjectLevelGLMSpec.m b/src/batches/stats/setBatchSubjectLevelGLMSpec.m index 427279bfb..c6f42a62b 100644 --- a/src/batches/stats/setBatchSubjectLevelGLMSpec.m +++ b/src/batches/stats/setBatchSubjectLevelGLMSpec.m @@ -185,7 +185,7 @@ matlabbatch{end + 1}.spm.stats.fmri_design = fmri_spec; else - node = model.get_root_node(); + node = bm.get_root_node(); fmri_spec.mask = {getInclusiveMask(opt, node.Name, BIDS, subLabel)}; matlabbatch{end + 1}.spm.stats.fmri_spec = fmri_spec; diff --git a/src/bids/getAvailableGroups.m b/src/bids/getAvailableGroups.m index 91b9e24cb..b7cb46153 100644 --- a/src/bids/getAvailableGroups.m +++ b/src/bids/getAvailableGroups.m @@ -1,6 +1,11 @@ function availableGroups = getAvailableGroups(opt, groupColumnHdr) % (C) Copyright 2024 bidspm developers + if isempty(groupColumnHdr) + availableGroups = {}; + return + end + tsvFile = fullfile(opt.dir.raw, 'participants.tsv'); assert(exist(tsvFile, 'file') == 2); diff --git a/src/bids_model/getContrastsList.m b/src/bids_model/getContrastsList.m index 323a800e5..331b3125e 100644 --- a/src/bids_model/getContrastsList.m +++ b/src/bids_model/getContrastsList.m @@ -48,7 +48,7 @@ % TODO relax those assumptions % assumptions - assert(model.validateGroupBy(node.Name participants)); + assert(model.validateGroupBy(node.Name, participants)); assert(node.Model.X == 1); contrastsList = getContrastsFromParentNode(model, node); diff --git a/src/bids_model/getContrastsListForFactorialDesign.m b/src/bids_model/getContrastsListForFactorialDesign.m index 2c460dfb1..b8ec49d00 100644 --- a/src/bids_model/getContrastsListForFactorialDesign.m +++ b/src/bids_model/getContrastsListForFactorialDesign.m @@ -41,7 +41,7 @@ % if no specific dummy contrasts mentioned also include all contrasts from previous levels % or if contrasts are mentioned we grab them if ~isfield(node.DummyContrasts, 'Contrasts') || isfield(node, 'Contrasts') - tmp = getContrastsList(bm, nodeName, columns); + tmp = getContrastsList(bm, nodeName, participants); for i = 1:numel(tmp) contrastsList{end + 1} = tmp{i}.Name; end diff --git a/src/stats/subject_level/specifyContrasts.m b/src/stats/subject_level/specifyContrasts.m index e45da936e..3e13cf3e9 100644 --- a/src/stats/subject_level/specifyContrasts.m +++ b/src/stats/subject_level/specifyContrasts.m @@ -113,7 +113,7 @@ return end - if isempty(model.Edges) + if isempty(bm.Edges) bm = bm.get_edges_from_nodes(); end diff --git a/src/workflows/roi/bidsRoiBasedGLM.m b/src/workflows/roi/bidsRoiBasedGLM.m index 4a515c5b3..e7ccb0482 100644 --- a/src/workflows/roi/bidsRoiBasedGLM.m +++ b/src/workflows/roi/bidsRoiBasedGLM.m @@ -79,11 +79,11 @@ if ~checkSpmMat(outputDir, opt, strict) continue end - spmFile = fullfile(outputDir, 'SPM.mat'); - load(spmFile, 'SPM'); + spmMatFile = fullfile(outputDir, 'SPM.mat'); + load(spmMatFile, 'SPM'); model = mardo(SPM); - eventSpec = getEventSpecificationRoiGlm(spmFile, opt.model.file); + eventSpec = getEventSpecificationRoiGlm(spmMatFile, opt.model.file); subTempDir = fullfile(tempDir, ['sub-' subLabel]); spm_mkdir(subTempDir); diff --git a/tests/tests_batches/preproc/test_setBatchGenerateT1map.m b/tests/tests_batches/preproc/test_setBatchGenerateT1map.m index 7a2911252..161d9ebb4 100644 --- a/tests/tests_batches/preproc/test_setBatchGenerateT1map.m +++ b/tests/tests_batches/preproc/test_setBatchGenerateT1map.m @@ -48,7 +48,7 @@ function test_setBatchGenerateT1map_warning() BIDS = getLayout(opt); matlabbatch = {}; - opt.verbosity = 2; + opt.verbosity = 1; assertWarning(@() setBatchGenerateT1map(matlabbatch, BIDS, opt, subLabel), ... 'setBatchGenerateT1map:missingNonBIDSMetadata'); diff --git a/tests/tests_bids/test_fileFilterForBold.m b/tests/tests_bids/test_fileFilterForBold.m index 50efff7f5..3de226fbb 100644 --- a/tests/tests_bids/test_fileFilterForBold.m +++ b/tests/tests_bids/test_fileFilterForBold.m @@ -13,7 +13,7 @@ function test_fileFilterForBold_events() opt.bidsFilterFile.bold = struct('modality', 'func', 'suffix', 'bold'); - opt.verbosity = 2; + opt.verbosity = 0; opt.taskName = 'foo'; opt.fwhm.func = 6; opt.space = {'IXI549Space'}; @@ -38,7 +38,7 @@ function test_fileFilterForBold_events_aroma() opt.bidsFilterFile.bold = struct('modality', 'func', ... 'suffix', 'bold', ... 'desc', {'smoothAROMAnonaggr'}); - opt.verbosity = 2; + opt.verbosity = 0; opt.taskName = 'foo'; opt.fwhm.func = 6; opt.space = {'MNI152NLin6Asym'}; @@ -61,7 +61,7 @@ function test_fileFilterForBold_events_aroma() function test_fileFilterForBold_confounds() opt.bidsFilterFile.bold = struct('modality', 'func', 'suffix', 'bold'); - opt.verbosity = 2; + opt.verbosity = 0; opt.taskName = 'foo'; opt.fwhm.func = 6; opt.space = {'IXI549Space'}; @@ -84,7 +84,7 @@ function test_fileFilterForBold_confounds() function test_fileFilterForBold_basic() opt.bidsFilterFile.bold = struct('modality', 'func', 'suffix', 'bold'); - opt.verbosity = 2; + opt.verbosity = 0; opt.taskName = 'foo'; opt.fwhm.func = 6; opt.space = {'IXI549Space'}; @@ -111,7 +111,7 @@ function test_fileFilterForBold_desc() opt.bidsFilterFile.bold = struct('modality', 'func', ... 'suffix', 'bold', ... 'desc', {'smoothAROMAnonaggr'}); - opt.verbosity = 2; + opt.verbosity = 0; opt.taskName = 'foo'; opt.fwhm.func = 6; opt.space = {'MNI152NLin6Asym'}; @@ -138,7 +138,7 @@ function test_fileFilterForBold_desc() function test_fileFilterForBold_no_smooth() opt.bidsFilterFile.bold = struct('modality', 'func', 'suffix', 'bold'); - opt.verbosity = 2; + opt.verbosity = 0; opt.taskName = 'foo'; opt.fwhm.func = 0; opt.space = {'IXI549Space'}; @@ -163,7 +163,7 @@ function test_fileFilterForBold_no_smooth() function test_fileFilterForBold_stc() opt.bidsFilterFile.bold = struct('modality', 'func', 'suffix', 'bold'); - opt.verbosity = 2; + opt.verbosity = 0; opt.taskName = 'foo'; opt.fwhm.func = 0; opt.space = {'IXI549Space'}; diff --git a/tests/tests_bids_model/test_getOptionsFromModel.m b/tests/tests_bids_model/test_getOptionsFromModel.m index fa71291e0..665a7ce1d 100644 --- a/tests/tests_bids_model/test_getOptionsFromModel.m +++ b/tests/tests_bids_model/test_getOptionsFromModel.m @@ -27,7 +27,7 @@ function test_getOptionsFromModel_no_model() opt.pipeline.type = 'stats'; opt.model.file = ''; opt.tolerant = true; - opt.verbosity = 3; + opt.verbosity = 1; opt.pipeline.isBms = false; assertExceptionThrown(@() getOptionsFromModel(opt), 'getOptionsFromModel:modelFileMissing'); @@ -79,7 +79,7 @@ function test_getOptionsFromModel_task() %% opt.taskName = {'foo'}; - opt.verbosity = 2; + opt.verbosity = 1; opt.tolerant = true; assertWarning(@() getOptionsFromModel(opt), 'getOptionsFromModel:modelOverridesOptions'); @@ -104,7 +104,7 @@ function test_getOptionsFromModel_subject() %% opt.subjects = '02'; - opt.verbosity = 2; + opt.verbosity = 1; assertWarning(@() getOptionsFromModel(opt), 'getOptionsFromModel:modelOverridesOptions'); @@ -147,7 +147,7 @@ function test_getOptionsFromModel_query() %% opt.query.acq = 'foo'; - opt.verbosity = 2; + opt.verbosity = 1; assertWarning(@() getOptionsFromModel(opt), 'getOptionsFromModel:modelOverridesOptions'); diff --git a/tests/tests_bids_model/test_validateGroupBy.m b/tests/tests_bids_model/test_validateGroupBy.m index 103950f16..3ca85b5e1 100644 --- a/tests/tests_bids_model/test_validateGroupBy.m +++ b/tests/tests_bids_model/test_validateGroupBy.m @@ -38,7 +38,7 @@ function test_validateGroupBy_subject() assertWarning(@()bm.validateGroupBy(nodeName), 'BidsModel:notImplemented'); end -function test_checkGroupBy_dataset() +function test_validateGroupBy_dataset() opt = setOptions('vismotion', {'01' 'ctrl01'}, 'pipelineType', 'stats'); @@ -62,7 +62,7 @@ function test_checkGroupBy_dataset() end -function test_checkGroupBy_dataset_group_from_participant() +function test_validateGroupBy_dataset_group_from_participant() opt = setOptions('vismotion', {'01' 'ctrl01'}, 'pipelineType', 'stats'); @@ -71,6 +71,6 @@ function test_checkGroupBy_dataset_group_from_participant() nodeName = bm.Nodes{3}.Name; bm.Nodes{3}.GroupBy = {'contrast', 'diagnostic'}; - bm.validateGroupBy(nodeName, {'diagnostic'}); + bm.validateGroupBy(nodeName, struct('diagnostic', {{'foo', 'bar'}})); end diff --git a/tests/tests_slow/tests_workflows/stats/test_bidsResults.m b/tests/tests_slow/tests_workflows/stats/test_bidsResults.m index b9a390034..253bf06fa 100644 --- a/tests/tests_slow/tests_workflows/stats/test_bidsResults.m +++ b/tests/tests_slow/tests_workflows/stats/test_bidsResults.m @@ -15,7 +15,7 @@ function test_bidsResults_no_results() skipIfOctave('mixed-string-concat warning thrown'); opt = setOptions('vismotion', '', 'pipelineType', 'stats'); - opt.verbosity = 2; + opt.verbosity = 1; opt = rmfield(opt, 'results'); diff --git a/tests/tests_slow/tests_workflows/test_renameSegmentParameter.m b/tests/tests_slow/tests_workflows/test_renameSegmentParameter.m index 933a74b15..a107a9f88 100644 --- a/tests/tests_slow/tests_workflows/test_renameSegmentParameter.m +++ b/tests/tests_slow/tests_workflows/test_renameSegmentParameter.m @@ -30,7 +30,7 @@ function test_renameSegmentParameter_basic() 'index_dependencies', false); opt.dryRun = false; - opt.verbosity = 2; + opt.verbosity = 0; segParamFiles = spm_select('FPListRec', tmpDir, '^.*_T1w_seg8.mat$'); assertEqual(size(segParamFiles, 1), 1); diff --git a/tests/tests_slow/tests_workflows/test_renameUnwarpParameter.m b/tests/tests_slow/tests_workflows/test_renameUnwarpParameter.m index 998d9ee1a..02df29050 100644 --- a/tests/tests_slow/tests_workflows/test_renameUnwarpParameter.m +++ b/tests/tests_slow/tests_workflows/test_renameUnwarpParameter.m @@ -25,7 +25,7 @@ function test_renameUnwarpParameter_basic() 'index_dependencies', false); opt.dryRun = false; - opt.verbosity = 2; + opt.verbosity = 0; unwarpParamFiles = spm_select('FPListRec', tmpDir, '^.*_bold_uw.mat$'); assertEqual(size(unwarpParamFiles, 1), 1); diff --git a/tests/tests_stats/unit_tests/test_getInclusiveMask.m b/tests/tests_stats/unit_tests/test_getInclusiveMask.m index 263d23a70..d59dc909f 100644 --- a/tests/tests_stats/unit_tests/test_getInclusiveMask.m +++ b/tests/tests_stats/unit_tests/test_getInclusiveMask.m @@ -15,7 +15,7 @@ function test_getInclusiveMask_too_many() opt = setOptions('vismotion', '01', 'useTempDir', true); - opt.verbosity = 2; + opt.verbosity = 1; opt.model.bm = BidsModel('file', opt.model.file); @@ -63,7 +63,7 @@ function test_getInclusiveMask_no_image() opt = setOptions('vismotion', '01'); - opt.verbosity = 2; + opt.verbosity = 1; opt.model.bm = BidsModel('file', opt.model.file);