From b30dcad4a1c05dd16be124085437b1f146d5f4e1 Mon Sep 17 00:00:00 2001 From: Remi Gau Date: Wed, 10 Jul 2024 00:57:39 +0200 Subject: [PATCH] [FIX] properly handle bids filter with bidspm copy (#1275) * fix 627 * do not expect space to be set * fix test --- src/IO/getData.m | 2 +- src/cli/cliCopy.m | 55 ++++----- src/cli/getOptionsFromCliArgument.m | 1 - src/workflows/bidsCopyInputFolder.m | 9 +- tests/tests_cli/test_bidspm_copy_raw.m | 110 ++++++++++++++++++ tests/tests_slow/tests_cli/test_bidspm_copy.m | 50 ++++++-- 6 files changed, 188 insertions(+), 39 deletions(-) diff --git a/src/IO/getData.m b/src/IO/getData.m index b29694a16..a770de80b 100644 --- a/src/IO/getData.m +++ b/src/IO/getData.m @@ -112,7 +112,7 @@ if ~anatOnly && isfield(opt, 'taskName') && ... ~any(ismember(opt.taskName, bids.query(BIDS, 'tasks'))) - msg = sprintf(['The task %s that you have asked for ', ... + msg = sprintf(['The task "%s" that you have asked for ', ... 'does not exist in this dataset.\n', ... 'List of tasks present in this dataset:\n%s'], ... strjoin(opt.taskName), ... diff --git a/src/cli/cliCopy.m b/src/cli/cliCopy.m index 20337b80a..3d9194472 100644 --- a/src/cli/cliCopy.m +++ b/src/cli/cliCopy.m @@ -18,41 +18,44 @@ function cliCopy(varargin) opt = checkOptions(opt); - opt.query.desc = {'preproc', 'brain'}; - opt.query.suffix = {'T1w', 'bold', 'mask'}; - if opt.anatOnly - opt.query.suffix = {'T1w', 'mask'}; - end + bidsFilterFile = getBidsFilterFile(args); + if isempty(opt.taskName) opt = rmfield(opt, 'taskName'); end - opt.query.space = opt.space; - bidsFilterFile = getBidsFilterFile(args); + if isempty(bidsFilterFile) + opt.query.desc = {'preproc', 'brain'}; + + opt.query.suffix = {'T1w', 'mask', 'bold', 'events'}; + + if opt.anatOnly + opt.query.suffix = {'T1w', 'mask'}; + end + + opt.query.space = opt.space; + + saveOptions(opt); + + bidsCopyInputFolder(opt, 'unzip', true, 'force', args.Results.force); + + else - if ~isempty(bidsFilterFile) suffixes = fieldnames(bidsFilterFile); - modalities = {}; for i = 1:numel(suffixes) - modalities{end + 1} = bidsFilterFile.(suffixes{i}).modality; %#ok<*AGROW> - if isfield(bidsFilterFile.(suffixes{i}), 'suffix') - opt.query.suffix = cat(2, ... - opt.query.suffix, ... - bidsFilterFile.(suffixes{i}).suffix); - end - if isfield(bidsFilterFile.(suffixes{i}), 'desc') - opt.query.desc = cat(2, ... - opt.query.desc, ... - bidsFilterFile.(suffixes{i}).desc); + + if opt.anatOnly && ~strcmp(bidsFilterFile.(suffixes{i}).modality, 'anat') + continue end - end - opt.query.modality = unique(modalities); - end - opt.query.suffix = unique(opt.query.suffix); - opt.query.desc = unique(opt.query.desc); - saveOptions(opt); + opt.query = bidsFilterFile.(suffixes{i}); - bidsCopyInputFolder(opt, 'unzip', true, 'force', args.Results.force); + saveOptions(opt); + + bidsCopyInputFolder(opt, 'unzip', true, 'force', args.Results.force); + + end + + end end diff --git a/src/cli/getOptionsFromCliArgument.m b/src/cli/getOptionsFromCliArgument.m index eaab7b5db..c776c7675 100644 --- a/src/cli/getOptionsFromCliArgument.m +++ b/src/cli/getOptionsFromCliArgument.m @@ -77,7 +77,6 @@ if opt.verbosity > 3 unfold(opt); end - unfold(opt); end diff --git a/src/workflows/bidsCopyInputFolder.m b/src/workflows/bidsCopyInputFolder.m index 08cec7e4f..3c721e5cf 100644 --- a/src/workflows/bidsCopyInputFolder.m +++ b/src/workflows/bidsCopyInputFolder.m @@ -80,9 +80,10 @@ function bidsCopyInputFolder(varargin) filter.sub = opt.subjects; if strcmp(filter.modality, 'func') - filter.task = opt.taskName; - if isempty(filter.task) + if ~isfield(opt, 'taskName') || isempty(opt.taskName) filter.task = bids.query(BIDS, 'tasks', filter); + else + filter.task = opt.taskName; end end @@ -116,7 +117,9 @@ function bidsCopyInputFolder(varargin) if isfield(filter, 'desc') filter = rmfield(filter, 'desc'); end - filter = rmfield(filter, 'space'); + if isfield(filter, 'space') + filter = rmfield(filter, 'space'); + end filter.suffix = {'regressors', 'timeseries', 'motion', 'outliers'}; bids.copy_to_derivative(BIDS, ... diff --git a/tests/tests_cli/test_bidspm_copy_raw.m b/tests/tests_cli/test_bidspm_copy_raw.m index 1b8babe95..499ebc66c 100644 --- a/tests/tests_cli/test_bidspm_copy_raw.m +++ b/tests/tests_cli/test_bidspm_copy_raw.m @@ -27,3 +27,113 @@ function test_copy_anat_only() assertEqual(numel(bids.query(BIDS, 'data')), 1); end + +function test_copy_no_task() + + inputPath = fullfile(getMoaeDir(), 'inputs', 'raw'); + + outputPath = tempName(); + + bidspm(inputPath, outputPath, 'subject', ... + 'action', 'copy', ... + 'verbosity', 0); + + BIDS = bids.layout(fullfile(outputPath, 'derivatives', 'bidspm-preproc'), ... + 'verbose', false, ... + 'use_schema', false); + + assertEqual(numel(bids.query(BIDS, 'data')), 3); + +end + +function test_copy_filter_suffix() + + inputPath = fullfile(getMoaeDir(), 'inputs', 'raw'); + + outputPath = tempName(); + + bidspm(inputPath, outputPath, 'subject', ... + 'action', 'copy', ... + 'bids_filter_file', struct('bold', struct('modality', 'func', ... + 'suffix', 'events')), ... + 'verbosity', 0); + + BIDS = bids.layout(fullfile(outputPath, 'derivatives', 'bidspm-preproc'), ... + 'verbose', false, ... + 'use_schema', false); + + assertEqual(numel(bids.query(BIDS, 'data')), 1); + +end + +function test_copy_filter_session() + + inputPath = getTestDataDir('raw'); + + outputPath = tempName(); + + bidspm(inputPath, outputPath, 'subject', ... + 'action', 'copy', ... + 'participant_label', {'01', 'ctrl01'}, ... + 'task', {'vismotion'}, ... + 'bids_filter_file', struct('bold', struct('modality', 'func', ... + 'suffix', 'events', ... + 'ses', '01')), ... + 'verbosity', 0); + + BIDS = bids.layout(fullfile(outputPath, 'derivatives', 'bidspm-preproc'), ... + 'verbose', false, ... + 'use_schema', false); + + assertEqual(numel(bids.query(BIDS, 'data')), 8); + +end + +function test_copy_filter_anat() + + inputPath = getTestDataDir('raw'); + + outputPath = tempName(); + + bidspm(inputPath, outputPath, 'subject', ... + 'action', 'copy', ... + 'participant_label', {'01', 'ctrl01'}, ... + 'task', {'vismotion'}, ... + 'bids_filter_file', struct('anat', struct('modality', 'anat', ... + 'suffix', 'T1w', ... + 'ses', '01')), ... + 'verbosity', 0); + + BIDS = bids.layout(fullfile(outputPath, 'derivatives', 'bidspm-preproc'), ... + 'verbose', false, ... + 'use_schema', false); + + assertEqual(numel(bids.query(BIDS, 'data')), 2); + +end + +function test_copy_several_filter() + + inputPath = getTestDataDir('raw'); + + outputPath = tempName(); + + bidspm(inputPath, outputPath, 'subject', ... + 'action', 'copy', ... + 'participant_label', {'01', 'ctrl01'}, ... + 'task', {'vismotion'}, ... + 'bids_filter_file', struct('anat', struct('modality', 'anat', ... + 'suffix', 'T1w', ... + 'ses', '01'), ... + 'bold', struct('modality', 'func', ... + 'suffix', 'events', ... + 'ses', '01')), ... + 'verbosity', 0); + + BIDS = bids.layout(fullfile(outputPath, 'derivatives', 'bidspm-preproc'), ... + 'verbose', false, ... + 'use_schema', false); + + assertEqual(numel(bids.query(BIDS, 'data')), 10); + +end diff --git a/tests/tests_slow/tests_cli/test_bidspm_copy.m b/tests/tests_slow/tests_cli/test_bidspm_copy.m index 5875439d8..74c382be8 100644 --- a/tests/tests_slow/tests_cli/test_bidspm_copy.m +++ b/tests/tests_slow/tests_cli/test_bidspm_copy.m @@ -9,7 +9,7 @@ initTestSuite; end -function test_copy_filter() +function test_copy_simple_filter() markTestAs('slow'); @@ -32,10 +32,12 @@ function test_copy_filter() end copyfile(sourceFile{1}, destFile{1}); - %% with simple filter file + % simple filter file bids_filter_file = fullfile(tempName(), 'bids_filter_file.json'); bids.util.jsonencode(bids_filter_file, ... - struct('bold', struct('modality', 'func'))); + struct('bold', struct('modality', 'func', ... + 'extension', '.nii.gz', ... + 'desc', {{'preproc', 'brain'}}))); % filter takes precedence over predefined opt.query % so anat should not be copied @@ -55,13 +57,45 @@ function test_copy_filter() 'verbose', false, ... 'use_schema', false); - assertEqual(numel(bids.query(BIDS, 'data')), 5); + assertEqual(numel(bids.query(BIDS, 'data')), 4); - %% now with more complex filter as struct + delete(destFile{1}); + +end + +function test_copy_complex_filter() + + markTestAs('slow'); + + inputPath = fullfile(getMoaeDir(), 'inputs', 'fmriprep'); + + % add dummy aroma file to input folder + BIDS = bids.layout(inputPath, ... + 'verbose', false, ... + 'use_schema', false); + sourceFile = bids.query(BIDS, 'data', ... + 'suffix', 'bold', ... + 'desc', 'preproc', ... + 'space', 'T1w'); + bf = bids.File(sourceFile{1}); + bf.entities.desc = 'smoothAROMAnonaggr'; + bf = bf.update; + destFile = bids.internal.file_utils(sourceFile, 'filename', bf.filename); + if exist(destFile{1}, 'file') + delete(destFile{1}); + end + copyfile(sourceFile{1}, destFile{1}); + + % more complex filter as struct bids_filter_file = struct('bold', struct('modality', 'func', ... 'suffix', 'bold', ... - 'desc', {'smoothAROMAnonaggr'; ... - 'preproc'})); + 'desc', {{'smoothAROMAnonaggr'; ... + 'preproc'}}), ... + 'anat', struct('modality', 'anat', ... + 'suffix', 't1w', ... + 'desc', 'preproc')); + + outputPath = tempName(); bidspm(inputPath, outputPath, 'subject', ... 'action', 'copy', ... @@ -74,7 +108,7 @@ function test_copy_filter() 'verbose', false, ... 'use_schema', false); - assertEqual(numel(bids.query(BIDS, 'data')), 6); + assertEqual(numel(bids.query(BIDS, 'data')), 4); delete(destFile{1});