From 877135919bb18dec4b1eb3bc3bf53c42f5ec11d5 Mon Sep 17 00:00:00 2001 From: Remi Gau Date: Thu, 25 Jan 2024 13:39:10 +0100 Subject: [PATCH] [ENH] remove checks for participants.tsv or samples.tsv in derivatives (#666) * no warning for missing participants.tsv in derivatives * do not copy participants.tsv to derivatives * skip for octave * skip on windows --- +bids/+util/tsvwrite.m | 2 +- +bids/copy_to_derivative.m | 19 ----------- +bids/layout.m | 26 +++++++++++---- tests/test_bids_model.m | 4 +-- tests/tests_layout/test_layout.m | 33 +++++++++++++++++-- tests/tests_layout/test_layout_derivatives.m | 4 +-- tests/tests_private/test_append_to_layout.m | 16 +++------ .../tests_private/test_list_all_trial_types.m | 6 ++-- tests/tests_private/test_parse_filename.m | 4 +-- tests/tests_private/test_return_file_index.m | 4 +-- tests/tests_utils/test_create_data_dict.m | 4 +-- .../test_create_participants_tsv.m | 4 +-- tests/tests_utils/test_create_readme.m | 4 +-- tests/tests_utils/test_create_sessions_tsv.m | 4 +-- tests/utils/skip_if_octave.m | 6 ++++ 15 files changed, 72 insertions(+), 68 deletions(-) create mode 100644 tests/utils/skip_if_octave.m diff --git a/+bids/+util/tsvwrite.m b/+bids/+util/tsvwrite.m index 49e84fd6..5a4a5d74 100644 --- a/+bids/+util/tsvwrite.m +++ b/+bids/+util/tsvwrite.m @@ -4,7 +4,7 @@ function tsvwrite(filename, var) % % USAGE:: % - % tsvwrite(f, var) + % tsvwrite(filename, var) % % :param filename: % :type filename: string diff --git a/+bids/copy_to_derivative.m b/+bids/copy_to_derivative.m index 2d36c15d..662d41b4 100644 --- a/+bids/copy_to_derivative.m +++ b/+bids/copy_to_derivative.m @@ -159,8 +159,6 @@ function copy_to_derivative(varargin) ds_desc.write(derivatives_folder); - copy_participants_tsv(BIDS, derivatives_folder, args); - % looping over selected files for iFile = 1:numel(data_list) copy_file(BIDS, derivatives_folder, data_list{iFile}, ... @@ -178,23 +176,6 @@ function copy_to_derivative(varargin) end -function copy_participants_tsv(BIDS, derivatives_folder, args) - % - % Very "brutal" approach where we copy the whole file - % - % TODO: if only certain subjects are copied only copy those entries from the TSV - % - - if ~isempty(BIDS.participants) - - src = fullfile(BIDS.pth, 'participants.tsv'); - target = fullfile(derivatives_folder, 'participants.tsv'); - - copy_tsv(src, target, args); - - end -end - function copy_tsv(src, target, args) flag = false; diff --git a/+bids/layout.m b/+bids/layout.m index 3e5d1100..142d7ead 100644 --- a/+bids/layout.m +++ b/+bids/layout.m @@ -138,9 +138,6 @@ % [sourcedata/] - ignore % [phenotype/] - BIDS.participants = []; - BIDS.participants = manage_tsv(BIDS.participants, BIDS.pth, 'participants.tsv', verbose); - BIDS = index_phenotype(BIDS); BIDS = index_root_directory(BIDS); @@ -220,10 +217,7 @@ BIDS = index_derivatives_dir(BIDS, index_derivatives, verbose); - if ismember('micr', bids.query(BIDS, 'modalities')) - BIDS.samples = []; - BIDS.samples = manage_tsv(BIDS.samples, BIDS.pth, 'samples.tsv', verbose); - end + BIDS = index_participants_and_sample(BIDS, verbose); end @@ -259,6 +253,24 @@ function handle_invalid_input(ME, root) end end +function BIDS = index_participants_and_sample(BIDS, verbose) + warn_for_missing_tsv = verbose; + if isfield(BIDS.description, 'DatasetType') && ... + strcmp(BIDS.description.DatasetType, 'derivative') + warn_for_missing_tsv = false; + end + + BIDS.participants = []; + BIDS.participants = manage_tsv(BIDS.participants, BIDS.pth, ... + 'participants.tsv', warn_for_missing_tsv); + + if ismember('micr', bids.query(BIDS, 'modalities')) + BIDS.samples = []; + BIDS.samples = manage_tsv(BIDS.samples, BIDS.pth, ... + 'samples.tsv', warn_for_missing_tsv); + end +end + function value = exclude(filter, entity, label) value = false; % skip if not included in filter diff --git a/tests/test_bids_model.m b/tests/test_bids_model.m index 63075faa..1f987ca3 100644 --- a/tests/test_bids_model.m +++ b/tests/test_bids_model.m @@ -180,9 +180,7 @@ function test_model_default_no_events() function test_model_validate() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); bm = bids.Model(); bm.Nodes{1} = rmfield(bm.Nodes{1}, 'Name'); diff --git a/tests/tests_layout/test_layout.m b/tests/tests_layout/test_layout.m index 65cff4f3..d8b1963a 100644 --- a/tests/tests_layout/test_layout.m +++ b/tests/tests_layout/test_layout.m @@ -6,6 +6,33 @@ initTestSuite; end +function test_warning_missing_participants_tsv() + + skip_if_octave('mixed-string-concat warning thrown'); + + bids_dir = fullfile(get_test_data_dir(), 'qmri_tb1tfl'); + assertWarning(@()bids.layout(bids_dir, ... + 'verbose', true), ... + 'layout:tsvMissing'); + +end + +function test_no_warning_missing_participants_tsv_derivatives() + + skip_if_octave('mixed-string-concat warning thrown'); + + bids_dir = fullfile(get_test_data_dir(), 'ds000001-fmriprep'); + try + assertWarning(@()bids.layout(bids_dir, ... + 'verbose', true, ... + 'use_schema', false), ... + 'layout:tsvMissing'); + catch ME + assert(strcmp(ME.identifier, 'moxunit:warningNotRaised')); + end + +end + function test_layout_do_not_include_empty_subject() if ispc @@ -33,8 +60,10 @@ function test_layout_do_not_include_empty_subject() function test_layout_do_not_include_empty_subject_warning() - if bids.internal.is_octave() || ispc - moxunit_throw_test_skipped_exception('Octave mixed-string-concat or fail on windows'); + skip_if_octave('mixed-string-concat warning thrown'); + if ispc + % TODO investigate + moxunit_throw_test_skipped_exception('fail on windows'); end bids_dir = fullfile(get_test_data_dir(), 'qmri_tb1tfl'); diff --git a/tests/tests_layout/test_layout_derivatives.m b/tests/tests_layout/test_layout_derivatives.m index ad024bf2..cb716fc1 100644 --- a/tests/tests_layout/test_layout_derivatives.m +++ b/tests/tests_layout/test_layout_derivatives.m @@ -125,9 +125,7 @@ function test_layout_warning_invalid_subfolder_struct_fieldname() invalid_subfolder = fullfile(get_test_data_dir(), '..', ... 'data', 'synthetic', 'derivatives', 'invalid_subfolder'); - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); assertWarning(@()bids.layout(invalid_subfolder, ... 'use_schema', false, ... diff --git a/tests/tests_private/test_append_to_layout.m b/tests/tests_private/test_append_to_layout.m index b107d151..57adfc96 100644 --- a/tests/tests_private/test_append_to_layout.m +++ b/tests/tests_private/test_append_to_layout.m @@ -13,9 +13,7 @@ function test_layout_missing_subgroup() synthetic_derivatives = fullfile(get_test_data_dir(), '..', ... 'data', 'synthetic', 'derivatives', 'manual'); - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); assertWarning(@()bids.layout(synthetic_derivatives, 'verbose', true), ... 'append_to_layout:unknownSuffix'); @@ -24,9 +22,7 @@ function test_layout_missing_subgroup() function test_append_to_layout_schema_unknown_entity() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); [subject, modality, schema, previous] = set_up('meg'); @@ -39,9 +35,7 @@ function test_append_to_layout_schema_unknown_entity() function test_append_to_layout_schema_unknown_extension() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); [subject, modality, schema, previous] = set_up('meg'); @@ -91,9 +85,7 @@ function test_append_to_layout_basic() function test_append_to_layout_schema_missing_required_entity() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); [subject, modality, schema, previous] = set_up('func'); diff --git a/tests/tests_private/test_list_all_trial_types.m b/tests/tests_private/test_list_all_trial_types.m index 5e323270..76efa290 100644 --- a/tests/tests_private/test_list_all_trial_types.m +++ b/tests/tests_private/test_list_all_trial_types.m @@ -32,9 +32,9 @@ function test_list_all_trial_types_warning() trial_type_list = bids.internal.list_all_trial_types(BIDS, {'not', 'a', 'task'}, ... 'verbose', false); assertEqual(trial_type_list, {}); - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + + skip_if_octave('mixed-string-concat warning thrown'); + assertWarning(@() bids.internal.list_all_trial_types(BIDS, {'not', 'a', 'task'}, ... 'verbose', true), ... 'list_all_trial_types:noEventsFile'); diff --git a/tests/tests_private/test_parse_filename.m b/tests/tests_private/test_parse_filename.m index 8f2a0c85..c9cf3642 100644 --- a/tests/tests_private/test_parse_filename.m +++ b/tests/tests_private/test_parse_filename.m @@ -8,9 +8,7 @@ function test_parse_filename_warnings() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); fields = {}; tolerant = true; diff --git a/tests/tests_private/test_return_file_index.m b/tests/tests_private/test_return_file_index.m index baeed7a9..6ce49a45 100644 --- a/tests/tests_private/test_return_file_index.m +++ b/tests/tests_private/test_return_file_index.m @@ -29,9 +29,7 @@ function test_return_file_index_basic() function test_return_file_index_warning() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); pth_bids_example = get_test_data_dir(); diff --git a/tests/tests_utils/test_create_data_dict.m b/tests/tests_utils/test_create_data_dict.m index 225e4278..af010f85 100644 --- a/tests/tests_utils/test_create_data_dict.m +++ b/tests/tests_utils/test_create_data_dict.m @@ -163,9 +163,7 @@ function test_create_data_dict_schema() function test_create_data_dict_warning - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); dataset = 'ds000248'; diff --git a/tests/tests_utils/test_create_participants_tsv.m b/tests/tests_utils/test_create_participants_tsv.m index c1e751c6..5f3a14c1 100644 --- a/tests/tests_utils/test_create_participants_tsv.m +++ b/tests/tests_utils/test_create_participants_tsv.m @@ -22,9 +22,7 @@ function test_create_participants_tsv_basic() function test_create_participants_tsv_already_exist() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); bids_path = fullfile(get_test_data_dir(), 'ds210'); diff --git a/tests/tests_utils/test_create_readme.m b/tests/tests_utils/test_create_readme.m index cbbd3037..5ee20b51 100644 --- a/tests/tests_utils/test_create_readme.m +++ b/tests/tests_utils/test_create_readme.m @@ -26,9 +26,7 @@ function test_create_readme_basic() function test_create_readme_warning_already_present() - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); bids_path = fullfile(get_test_data_dir(), 'ds116'); diff --git a/tests/tests_utils/test_create_sessions_tsv.m b/tests/tests_utils/test_create_sessions_tsv.m index 1443076c..bc41f9c2 100644 --- a/tests/tests_utils/test_create_sessions_tsv.m +++ b/tests/tests_utils/test_create_sessions_tsv.m @@ -18,9 +18,7 @@ function test_create_sessions_tsv_no_session() validate_dataset(bids_path); - if bids.internal.is_octave() - moxunit_throw_test_skipped_exception('Octave:mixed-string-concat warning thrown'); - end + skip_if_octave('mixed-string-concat warning thrown'); assertWarning(@() bids.util.create_sessions_tsv(bids_path, 'verbose', true), ... 'create_sessions_tsv:noSessionInDataset'); diff --git a/tests/utils/skip_if_octave.m b/tests/utils/skip_if_octave.m new file mode 100644 index 00000000..d176c9c1 --- /dev/null +++ b/tests/utils/skip_if_octave.m @@ -0,0 +1,6 @@ +function skip_if_octave(msg) + if bids.internal.is_octave() + moxunit_throw_test_skipped_exception(['Octave:', msg]); + end + +end