From 6dbd28a134df3d0bec1d64693a4d26f19f2ed600 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Wed, 9 Dec 2020 14:42:03 -0500 Subject: [PATCH 01/78] Adds `add_field_to_json` to utils.py It also adds the corresponding unit test --- heudiconv/tests/test_utils.py | 27 +++++++++++++++++++++++++++ heudiconv/utils.py | 26 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/heudiconv/tests/test_utils.py b/heudiconv/tests/test_utils.py index b8c8c160..e4fcf034 100644 --- a/heudiconv/tests/test_utils.py +++ b/heudiconv/tests/test_utils.py @@ -10,6 +10,7 @@ load_json, create_tree, save_json, + add_field_to_json, get_datetime, JSONDecodeError) @@ -88,6 +89,32 @@ def test_load_json(tmpdir, caplog): assert load_json(valid_json_file) == vcontent +def test_add_field_to_json(tmpdir): + """ + Test utils.add_field_to_json() + """ + dummy_json_file = str(tmpdir / 'dummy.json') + some_content = {"name": "Jason", "age": 30, "city": "New York"} + save_json(dummy_json_file, some_content, pretty=True) + + added_content = {"LastName": "Bourne", + "Movies": [ + "The Bourne Identity", + "The Bourne Supremacy", + "The Bourne Ultimatum", + "The Bourne Legacy", + "Jason Bourne" + ] + } + add_field_to_json(dummy_json_file, added_content) + + # check that it was added: + with open(dummy_json_file) as f: + data = json.load(f) + some_content.update(added_content) + assert data == some_content + + def test_get_datetime(): """ Test utils.get_datetime() diff --git a/heudiconv/utils.py b/heudiconv/utils.py index f2631757..f96244c9 100644 --- a/heudiconv/utils.py +++ b/heudiconv/utils.py @@ -264,6 +264,32 @@ def json_dumps_pretty(j, indent=2, sort_keys=True): return js_ +def add_field_to_json(json_file, field_and_value): + """ + Adds a given field (and its value) to a json file + + Parameters: + ----------- + json_file : str or Path + path for the corresponding json file + field_and_value : dict + pair of "key": "value" to add to the json file + """ + for key, value in field_and_value.items(): + lgr.debug( + 'File: "{f}": Adding {k}: {v}'.format( + f=json_file, + k=key, + v=value, + ) + ) + + with open(json_file) as f: + data = json.load(f) + data.update(field_and_value) + save_json(json_file, data, pretty=True) + + def treat_infofile(filename): """Tune up generated .json file (slim down, pretty-print for humans). """ From d5f01b908cdf1142151db3542cc09fcea84322cb Mon Sep 17 00:00:00 2001 From: pvelasco Date: Fri, 11 Dec 2020 16:54:46 -0500 Subject: [PATCH 02/78] ENH: Adds `populate_intended_for` It adds a function to populate the "IntendedFor" field to the fmap jsons. It also adds the corresponding test. Minor modification to `create_tree` for the case of a `name` ending in `.json`. --- heudiconv/bids.py | 123 ++++++++++++++++++++++++++ heudiconv/tests/test_bids.py | 161 +++++++++++++++++++++++++++++++++++ heudiconv/utils.py | 6 +- 3 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 heudiconv/tests/test_bids.py diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 409ffa8f..cdc03555 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -19,6 +19,7 @@ save_json, create_file_if_missing, json_dumps_pretty, + add_field_to_json, set_readonly, is_readonly, get_datetime, @@ -457,3 +458,125 @@ def convert_sid_bids(subject_id): lgr.warning('{0} contained nonalphanumeric character(s), subject ' 'ID was cleaned to be {1}'.format(subject_id, sid)) return sid, subject_id + + +def populate_intended_for(path_to_bids_session): + """ + Adds the 'IntendedFor' field to the fmap .json files in a session folder. + It goes through the session folders and checks what runs have the same + 'ShimSetting' as the fmaps. If there is no 'ShimSetting' field in the json + file, we'll use the folder name ('func', 'dwi', 'anat') and see which fmap + with a matching '_acq' entity. + + If several fmap runs have the same 'ShimSetting' (or '_acq'), it will use + the first one. Because fmaps come in groups (with reversed PE polarity, + or magnitude/phase), it adds the same runs to the 'IntendedFor' of the + corresponding fmaps by checking the '_acq' and '_run' entities. + + Note: the logic behind the way we decide how to populate the "IntendedFor" + is: we want all images in the session (except for the fmap images + themselves) to have AT MOST one fmap. (That is, a pair of SE EPI with + reversed polarities, or a magnitude a phase field map). If there are more + than one fmap (more than a fmap pair) with the same acquisition parameters + as, say, a functional run, we will just assign that run to the FIRST pair, + while leaving the other fmap pairs without any assigned images. If the + user's intentions were different, he/she will have to manually edit the + fmap json files. + + Parameters: + ---------- + path_to_bids_session : str or os.path + path to the session folder (or to the subject folder, if there are no + sessions). + """ + lgr.info('') + lgr.info('Adding "IntendedFor" to the fieldmaps in {}.'.format(path_to_bids_session)) + + shim_key = 'ShimSetting' + lgr.debug('shim_key: {}'.format(shim_key)) + + # Resolve path (eliminate '..') + path_to_bids_session = op.abspath(path_to_bids_session) + + # get the BIDS folder (if "data_folder" includes the session, remove it): + if op.basename(path_to_bids_session).startswith('ses-'): + bids_folder = op.dirname(path_to_bids_session) + else: + bids_folder = path_to_bids_session + + fmap_dir = op.join(path_to_bids_session, 'fmap') + if not op.exists(fmap_dir): + lgr.warning('Fmap folder not found in {}.'.format(path_to_bids_session)) + lgr.warning('We cannot add the IntendedFor field') + return + + # Get a list of all fmap json files in the session: + # (we will remove elements later on, so don't just iterate) + fmap_jsons = sorted([j for j in glob(op.join(path_to_bids_session, 'fmap/*.json'))]) + + # Get a set with all non-fmap json files in the session (set is easier): + # We also exclude the SBRef files. + session_jsons = set( + j for j in glob(op.join(path_to_bids_session, '*/*.json')) if not ( + j in fmap_jsons + # j[:-5] removes the '.json' from the end + or j[-5].endswith('_sbref') + ) + ) + + # Loop through all the fmap json files and, for each one, find which other + # non-fmap images in the session have the same shim settings. Those that + # match are added to the intended_for list and removed from the list of + # non-fmap json files in the session (since they have already assigned to + # a fmap). + # After finishing with all the non-fmap images in the session, we go back + # to the fmap json file list, and find any other fmap json files of the + # same acquisition type and run number (because fmaps have several files: + # normal- and reversed-polarity, or magnitude and phase, etc.) We add the + # same IntendedFor list to those other corresponding fmap json files, and + # remove them from the list of available fmap json files. + # Once we have gone through all the fmap json files, we are done. + jsons_accounted_for = set() + for fm_json in fmap_jsons: + lgr.debug('Looking for runs for {}'.format(fm_json)) + data = load_json(fm_json) + if shim_key in data.keys(): + fm_shims = data[shim_key] + else: + fm_shims = fm_json.name.split('_acq-')[1].split('_')[0] + if fm_shims.lower() == 'fmri': + fm_shims = 'func' + + intended_for = [] + for image_json in session_jsons: + data = load_json(image_json) + if shim_key in data.keys(): + image_shims = data[shim_key] + else: + # we just use the acquisition type (the name of the folder: + # 'anat', 'func', ...) + image_shims = op.basename(op.dirname(image_json)) + if image_shims == fm_shims: + # BIDS specifies that the intended for are: + # - **image** files + # - path relative to the **subject level** + image_json_relative_path = op.relpath(image_json, start=bids_folder) + # image_json_relative_path[:-5] removes the '.json' extension: + intended_for.append( + image_json_relative_path[:-5] + '.nii.gz' + ) + jsons_accounted_for.add(image_json) + if len(intended_for) > 0: + fm_json_name = op.basename(fm_json) + # get from "_acq-"/"_run-" to the next "_": + acq_str = '_acq-' + fm_json_name.split('_acq-')[1].split('_')[0] + run_str = '_run-' + fm_json_name.split('_run-')[1].split('_')[0] + # Loop through all the files that have the same "acq-" and "run-" + intended_for = sorted([str(f) for f in intended_for]) + for other_fm_json in glob(op.join(path_to_bids_session, 'fmap/*' + acq_str + '*' + run_str + '*.json')): + # add the IntendedFor field to the json file: + add_field_to_json(other_fm_json, {"IntendedFor": intended_for}) + fmap_jsons.remove(other_fm_json) + # Remove the runs accounted for from the session_jsons list, so that + # we don't assign another fmap to this image: + session_jsons -= jsons_accounted_for diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py new file mode 100644 index 00000000..c88ce3a4 --- /dev/null +++ b/heudiconv/tests/test_bids.py @@ -0,0 +1,161 @@ +import json +import os +import os.path as op +from random import random + +from heudiconv.utils import ( + load_json, + create_tree, +) +from heudiconv.bids import populate_intended_for + +import pytest + + +def create_dummy_bids_session(session_path): + """ + Creates a dummy BIDS session, with slim json files and empty nii.gz + Parameters: + ---------- + session_path : str or os.path + path to the session (or subject) level folder + """ + session_parent, session_basename = op.split(session_path) + if session_basename.startswith('ses-'): + subj_folder = session_parent + prefix = op.split(session_parent)[1] + '_' + session_basename + else: + subj_folder = session_path + prefix = session_basename + + # Generate some random ShimSettings: + shim_length = 6 + dwi_shims = ['{0:.4f}'.format(random()) for i in range(shim_length)] + func_shims_A = ['{0:.4f}'.format(random()) for i in range(shim_length)] + func_shims_B = ['{0:.4f}'.format(random()) for i in range(shim_length)] + + # Dict with the file structure for the session: + # -anat: + anat_struct = { + '{p}_{m}.nii.gz'.format(p=prefix, m=mod): '' for mod in ['T1w', 'T2w'] + } + anat_struct.update({ + # empty json: + '{p}_{m}.json'.format(p=prefix, m=mod): {} for mod in ['T1w', 'T2w'] + }) + # -dwi: + dwi_struct = { + '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=prefix, r=runNo): '' for runNo in [1, 2] + } + dwi_struct.update({ + '{p}_acq-A_run-{r}_dwi.json'.format(p=prefix, r=runNo): {'ShimSetting': dwi_shims} for runNo in [1, 2] + }) + # -func: + func_struct = { + '{p}_acq-{a}_bold.nii.gz'.format(p=prefix, a=acq): '' for acq in ['A', 'B', 'unmatched'] + } + func_struct.update({ + '{p}_acq-A_bold.json'.format(p=prefix): {'ShimSetting': func_shims_A}, + '{p}_acq-B_bold.json'.format(p=prefix): {'ShimSetting': func_shims_B}, + '{p}_acq-unmatched_bold.json'.format(p=prefix): { + 'ShimSetting': ['{0:.4f}'.format(random()) for i in range(shim_length)] + }, + }) + # -fmap: + fmap_struct = { + '{p}_acq-{a}_dir-{d}_run-{r}_epi.nii.gz'.format(p=prefix, a=acq, d=d, r=r): '' + for acq in ['dwi', 'fMRI'] + for d in ['AP', 'PA'] + for r in [1, 2] + } + fmap_struct.update({ + '{p}_acq-dwi_dir-{d}_run-{r}_epi.json'.format(p=prefix, d=d, r=r): {'ShimSetting': dwi_shims} + for d in ['AP', 'PA'] + for r in [1, 2] + }) + fmap_struct.update({ + '{p}_acq-fMRI_dir-{d}_run-{r}_epi.json'.format(p=prefix, d=d, r=r): {'ShimSetting': shims} + for r, shims in {'1': func_shims_A, '2': func_shims_B}.items() + for d in ['AP', 'PA'] + }) + # structure for the full session: + session_struct = { + 'anat': anat_struct, + 'dwi': dwi_struct, + 'func': func_struct, + 'fmap': fmap_struct + } + + create_tree(session_path, session_struct) + return session_struct + + +# Test two scenarios: +# -study without sessions +# -study with sessions +# The "expected_prefix" (the beginning of the path to the "IntendedFor") +# should be relative to the subject level +@pytest.mark.parametrize( + "folder, expected_prefix", [ + ('no_sessions/sub-1', ''), + ('sessions/sub-1/ses-pre', 'ses-pre') + ] +) +def test_populate_intended_for(tmpdir, folder, expected_prefix): + """ + Test populate_intended_for. + Parameters: + ---------- + tmpdir + folder : str or os.path + path to BIDS study to be simulated, relative to tmpdir + expected_prefix : str + expected start of the "IntendedFor" elements + """ + + session_folder = op.join(tmpdir, folder) + session_struct = create_dummy_bids_session(session_folder) + populate_intended_for(session_folder) + + run_prefix = 'sub-1' + ('_' + expected_prefix if expected_prefix else '') + # dict, with fmap names as keys and the expected "IntendedFor" as values. + expected_result = { + '{p}_acq-dwi_dir-{d}_run-{r}_epi.json'.format(p=run_prefix, d=d, r=runNo): + intended_for + # (runNo=1 goes with the long list, runNo=2 goes with None): + for runNo, intended_for in zip( + [1, 2], + [[op.join(expected_prefix, 'dwi', '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=run_prefix, r=r)) for r in [1,2]], + None] + ) + for d in ['AP', 'PA'] + } + expected_result.update( + { + '{p}_acq-fMRI_dir-{d}_run-{r}_epi.json'.format(p=run_prefix, d=d, r=runNo): + [ + op.join(expected_prefix, + 'func', + '{p}_acq-{a}_bold.nii.gz'.format(p=run_prefix, a=acq)) + ] + # runNo=1 goes with acq='A'; runNo=2 goes with acq='B' + for runNo, acq in zip([1, 2], ['A', 'B']) + for d in['AP', 'PA'] + } + ) + + # Now, loop through the jsons in the fmap folder and make sure it matches + # the expected result: + fmap_folder = op.join(session_folder, 'fmap') + for j in session_struct['fmap'].keys(): + if j.endswith('.json'): + assert j in expected_result.keys() + data = load_json(op.join(fmap_folder, j)) + if expected_result[j]: + assert data['IntendedFor'] == expected_result[j] + # Also, make sure the run with random shims is not here: + # (It is assured by the assert above, but let's make it + # explicit) + assert '{p}_acq-unmatched_bold.nii.gz'.format(p=run_prefix) not in data['IntendedFor'] + else: + assert 'IntendedFor' not in data.keys() diff --git a/heudiconv/utils.py b/heudiconv/utils.py index f96244c9..13bd3808 100644 --- a/heudiconv/utils.py +++ b/heudiconv/utils.py @@ -503,7 +503,11 @@ def create_tree(path, tree, archives_leading_dir=True): executable = False name = file_ full_name = op.join(path, name) - if isinstance(load, (tuple, list, dict)): + if name.endswith('.json') and isinstance(load, dict): + # (For a json file, we expect the content to be a dictionary, so + # don't continue creating a tree, but just write dict to file) + save_json(full_name, load, indent=4) + elif isinstance(load, (tuple, list, dict)): # if name.endswith('.tar.gz') or name.endswith('.tar') or name.endswith('.zip'): # create_tree_archive(path, name, load, archives_leading_dir=archives_leading_dir) # else: From 7117659ba3ad22d831f86efd5f7de40ce6bae60e Mon Sep 17 00:00:00 2001 From: pvelasco Date: Mon, 14 Dec 2020 15:46:03 -0500 Subject: [PATCH 03/78] Adds `populate_intended_for` to `process_extra_commands` It also adds the corresponding test --- heudiconv/main.py | 12 +++++++++--- heudiconv/tests/test_main.py | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/heudiconv/main.py b/heudiconv/main.py index c9e8ac4c..07347e83 100644 --- a/heudiconv/main.py +++ b/heudiconv/main.py @@ -3,7 +3,7 @@ import sys from . import __version__, __packagename__ -from .bids import populate_bids_templates, tuneup_bids_json_files +from .bids import populate_bids_templates, tuneup_bids_json_files, populate_intended_for from .convert import prep_conversion from .due import due, Doi from .parser import get_study_sessions @@ -47,13 +47,13 @@ def process_extra_commands(outdir, command, files, dicom_dir_template, heuristic, session, subjs, grouping): """ Perform custom command instead of regular operations. Supported commands: - ['treat-json', 'ls', 'populate-templates'] + ['treat-json', 'ls', 'populate-templates', 'populate_intended_for'] Parameters ---------- outdir : str Output directory - command : {'treat-json', 'ls', 'populate-templates'} + command : {'treat-json', 'ls', 'populate-templates', 'populate_intended_for'} Heudiconv command to run files : list of str List of files @@ -108,6 +108,12 @@ def process_extra_commands(outdir, command, files, dicom_dir_template, ensure_heuristic_arg(heuristic) from .utils import get_heuristic_description print(get_heuristic_description(heuristic, full=True)) + elif command == 'populate_intended_for': + for subj in subjs: + session_path = op.join(outdir, 'sub-' + subj) + if session: + session_path = op.join(session_path, 'ses-' + session) + populate_intended_for(session_path) else: raise ValueError("Unknown command %s" % command) return diff --git a/heudiconv/tests/test_main.py b/heudiconv/tests/test_main.py index d13f02ef..4568e00a 100644 --- a/heudiconv/tests/test_main.py +++ b/heudiconv/tests/test_main.py @@ -1,7 +1,8 @@ # TODO: break this up by modules from heudiconv.cli.run import main as runner -from heudiconv.main import workflow +from heudiconv.main import (workflow, + process_extra_commands) from heudiconv import __version__ from heudiconv.utils import (create_file_if_missing, set_readonly, @@ -290,3 +291,33 @@ def test_no_etelemetry(): with patch.dict('sys.modules', {'etelemetry': None}): workflow(outdir='/dev/null', command='ls', heuristic='reproin', files=[]) + + +# Test two scenarios: +# -study without sessions +# -study with sessions +# The "expected_folder" is the session folder without the tmpdir +@pytest.mark.parametrize( + "session, expected_folder", [ + ('', '/foo/sub-{sID}'), + ('pre', '/foo/sub-{sID}/ses-pre') + ] +) +def test_populate_intended_for(session, expected_folder, capfd): + """ + Tests for "process_extra_commands" when the command is + 'populate_intended_for' + """ + # Because the function .utils.populate_intended_for already has its own + # tests, here we just test that "process_extra_commands", when 'command' + # is 'populate_intended_for' does what we expect (loop through the list of + # subjects and calls 'populate_intended_for'). We call it using folders + # that don't exist, and check that the output is the expected. + bids_folder = expected_folder.split('sub-')[0] + subjects = ['1', '2'] + process_extra_commands(bids_folder, 'populate_intended_for', [], '', + '', session, subjects, None) + captured_output = capfd.readouterr().err + for s in subjects: + expected_info = 'Adding "IntendedFor" to the fieldmaps in ' + expected_folder.format(sID=s) + assert expected_info in captured_output From 5c4ff83299813ca0717bba03cce408d913638b15 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Thu, 17 Dec 2020 15:48:15 -0500 Subject: [PATCH 04/78] BF: acq_str/run_str will not break if no acq/run label present Also, fixes minor bug in ignoring `_sbref` images. --- heudiconv/bids.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index cdc03555..5346f3c1 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -520,7 +520,7 @@ def populate_intended_for(path_to_bids_session): j for j in glob(op.join(path_to_bids_session, '*/*.json')) if not ( j in fmap_jsons # j[:-5] removes the '.json' from the end - or j[-5].endswith('_sbref') + or j[:-5].endswith('_sbref') ) ) @@ -568,9 +568,11 @@ def populate_intended_for(path_to_bids_session): jsons_accounted_for.add(image_json) if len(intended_for) > 0: fm_json_name = op.basename(fm_json) - # get from "_acq-"/"_run-" to the next "_": - acq_str = '_acq-' + fm_json_name.split('_acq-')[1].split('_')[0] - run_str = '_run-' + fm_json_name.split('_run-')[1].split('_')[0] + # get and labels: + acq_match = re.findall('([/_]acq-([a-zA-Z0-9]*))', fm_json_name) + acq_str = acq_match[0][0] if acq_match else '' + run_match = re.findall('([/_]run-([a-zA-Z0-9]*))', fm_json_name) + run_str = run_match[0][0] if run_match else '' # Loop through all the files that have the same "acq-" and "run-" intended_for = sorted([str(f) for f in intended_for]) for other_fm_json in glob(op.join(path_to_bids_session, 'fmap/*' + acq_str + '*' + run_str + '*.json')): From 77d63f1347a6a018c99c278244bdbb102070cd60 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Thu, 17 Dec 2020 15:52:04 -0500 Subject: [PATCH 05/78] RF: `populate-intended-for` command uses dashes (`-`) ...rather than underscores (`_`) for consistency with other commands --- heudiconv/cli/run.py | 1 + heudiconv/main.py | 2 +- heudiconv/tests/test_main.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) mode change 100755 => 100644 heudiconv/cli/run.py diff --git a/heudiconv/cli/run.py b/heudiconv/cli/run.py old mode 100755 new mode 100644 index 8a381829..42bc85d0 --- a/heudiconv/cli/run.py +++ b/heudiconv/cli/run.py @@ -136,6 +136,7 @@ def get_parser(): 'heuristics', 'heuristic-info', 'ls', 'populate-templates', 'sanitize-jsons', 'treat-jsons', + 'populate-intended-for' ), help='Custom action to be performed on provided files instead of ' 'regular operation.') diff --git a/heudiconv/main.py b/heudiconv/main.py index 07347e83..f37b455d 100644 --- a/heudiconv/main.py +++ b/heudiconv/main.py @@ -108,7 +108,7 @@ def process_extra_commands(outdir, command, files, dicom_dir_template, ensure_heuristic_arg(heuristic) from .utils import get_heuristic_description print(get_heuristic_description(heuristic, full=True)) - elif command == 'populate_intended_for': + elif command == 'populate-intended-for': for subj in subjs: session_path = op.join(outdir, 'sub-' + subj) if session: diff --git a/heudiconv/tests/test_main.py b/heudiconv/tests/test_main.py index 4568e00a..8621979f 100644 --- a/heudiconv/tests/test_main.py +++ b/heudiconv/tests/test_main.py @@ -315,7 +315,7 @@ def test_populate_intended_for(session, expected_folder, capfd): # that don't exist, and check that the output is the expected. bids_folder = expected_folder.split('sub-')[0] subjects = ['1', '2'] - process_extra_commands(bids_folder, 'populate_intended_for', [], '', + process_extra_commands(bids_folder, 'populate-intended-for', [], '', '', session, subjects, None) captured_output = capfd.readouterr().err for s in subjects: From 42d3c20a0c378acc13e55b42aa7c16605124f1bd Mon Sep 17 00:00:00 2001 From: pvelasco Date: Thu, 17 Dec 2020 15:53:32 -0500 Subject: [PATCH 06/78] RF: Prepares way for further `populate_intended_for` unit tests For the case magnitude/phase fmaps. --- heudiconv/tests/test_bids.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py index c88ce3a4..4412655c 100644 --- a/heudiconv/tests/test_bids.py +++ b/heudiconv/tests/test_bids.py @@ -11,8 +11,8 @@ import pytest - -def create_dummy_bids_session(session_path): +# TODO: Do the same with a GRE fmap (magnitude/phase, etc.) +def create_dummy_pepolar_bids_session(session_path): """ Creates a dummy BIDS session, with slim json files and empty nii.gz Parameters: @@ -114,7 +114,7 @@ def test_populate_intended_for(tmpdir, folder, expected_prefix): """ session_folder = op.join(tmpdir, folder) - session_struct = create_dummy_bids_session(session_folder) + session_struct = create_dummy_pepolar_bids_session(session_folder) populate_intended_for(session_folder) run_prefix = 'sub-1' + ('_' + expected_prefix if expected_prefix else '') From 0d821d5eb6c76b81d938187149814c75c6b54cd6 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Thu, 17 Dec 2020 17:02:11 -0500 Subject: [PATCH 07/78] Corrects usage of `populate-intended-for` in allowed commands --- heudiconv/main.py | 4 ++-- heudiconv/tests/test_main.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/heudiconv/main.py b/heudiconv/main.py index f37b455d..a6912f05 100644 --- a/heudiconv/main.py +++ b/heudiconv/main.py @@ -47,13 +47,13 @@ def process_extra_commands(outdir, command, files, dicom_dir_template, heuristic, session, subjs, grouping): """ Perform custom command instead of regular operations. Supported commands: - ['treat-json', 'ls', 'populate-templates', 'populate_intended_for'] + ['treat-json', 'ls', 'populate-templates', 'populate-intended-for'] Parameters ---------- outdir : str Output directory - command : {'treat-json', 'ls', 'populate-templates', 'populate_intended_for'} + command : {'treat-json', 'ls', 'populate-templates', 'populate-intended-for'} Heudiconv command to run files : list of str List of files diff --git a/heudiconv/tests/test_main.py b/heudiconv/tests/test_main.py index 8621979f..bcc07a1b 100644 --- a/heudiconv/tests/test_main.py +++ b/heudiconv/tests/test_main.py @@ -306,7 +306,7 @@ def test_no_etelemetry(): def test_populate_intended_for(session, expected_folder, capfd): """ Tests for "process_extra_commands" when the command is - 'populate_intended_for' + 'populate-intended-for' """ # Because the function .utils.populate_intended_for already has its own # tests, here we just test that "process_extra_commands", when 'command' From 34b2accf53bf222f886454d5d80709901882f537 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Fri, 18 Dec 2020 15:58:15 -0500 Subject: [PATCH 08/78] BF: Fixes broken test for `populate_intended_for` in Python3.5 --- heudiconv/tests/test_bids.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py index 4412655c..d6188911 100644 --- a/heudiconv/tests/test_bids.py +++ b/heudiconv/tests/test_bids.py @@ -113,7 +113,7 @@ def test_populate_intended_for(tmpdir, folder, expected_prefix): expected start of the "IntendedFor" elements """ - session_folder = op.join(tmpdir, folder) + session_folder = op.join(str(tmpdir), folder) session_struct = create_dummy_pepolar_bids_session(session_folder) populate_intended_for(session_folder) From 7a01b5f6b57c9749d50b017b9751b340457c8b6e Mon Sep 17 00:00:00 2001 From: pvelasco Date: Fri, 18 Dec 2020 18:05:06 -0500 Subject: [PATCH 09/78] RF: make `get_shim_setting` a separate function It also adds the corresponding unit tests --- heudiconv/bids.py | 52 ++++++++++++++++++++++++------------ heudiconv/tests/test_bids.py | 52 +++++++++++++++++++++++++++++------- 2 files changed, 78 insertions(+), 26 deletions(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 6397519f..42e430d2 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -47,6 +47,9 @@ class BIDSError(Exception): BIDS_VERSION = "1.4.1" +SHIM_KEY = 'ShimSetting' +lgr.debug('shim_key: {}'.format(SHIM_KEY)) + def populate_bids_templates(path, defaults={}): """Premake BIDS text files with templates""" @@ -460,6 +463,36 @@ def convert_sid_bids(subject_id): return sid, subject_id +def get_shim_setting(json_file): + """ + Gets the "ShimSetting" field from a json_file. + If not present: + - for fmap files: use the entity from the file name + - for other files: use the folder name ('anat', 'func', 'dwi', ...) + + Parameters: + ---------- + json_file : str + + Returns: + ------- + str with "ShimSetting" value or entity + """ + data = load_json(json_file) + if SHIM_KEY in data.keys(): + shims = data[SHIM_KEY] + else: + modality = op.basename(op.dirname(json_file)) + if modality == 'fmap': + # extract the entity: + shims = re.search('(?<=[/_]acq-)\w+',json_file).group(0).split('_')[0] + if shims.lower() in ['fmri', 'bold']: + shims = 'func' + else: + shims = modality + return shims + + def populate_intended_for(path_to_bids_session): """ Adds the 'IntendedFor' field to the fmap .json files in a session folder. @@ -492,9 +525,6 @@ def populate_intended_for(path_to_bids_session): lgr.info('') lgr.info('Adding "IntendedFor" to the fieldmaps in {}.'.format(path_to_bids_session)) - shim_key = 'ShimSetting' - lgr.debug('shim_key: {}'.format(shim_key)) - # Resolve path (eliminate '..') path_to_bids_session = op.abspath(path_to_bids_session) @@ -539,23 +569,11 @@ def populate_intended_for(path_to_bids_session): jsons_accounted_for = set() for fm_json in fmap_jsons: lgr.debug('Looking for runs for {}'.format(fm_json)) - data = load_json(fm_json) - if shim_key in data.keys(): - fm_shims = data[shim_key] - else: - fm_shims = fm_json.name.split('_acq-')[1].split('_')[0] - if fm_shims.lower() == 'fmri': - fm_shims = 'func' + fm_shims = get_shim_setting(fm_json) intended_for = [] for image_json in session_jsons: - data = load_json(image_json) - if shim_key in data.keys(): - image_shims = data[shim_key] - else: - # we just use the acquisition type (the name of the folder: - # 'anat', 'func', ...) - image_shims = op.basename(op.dirname(image_json)) + image_shims = get_shim_setting(image_json) if image_shims == fm_shims: # BIDS specifies that the intended for are: # - **image** files diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py index d6188911..c547761e 100644 --- a/heudiconv/tests/test_bids.py +++ b/heudiconv/tests/test_bids.py @@ -5,16 +5,53 @@ from heudiconv.utils import ( load_json, + save_json, create_tree, ) -from heudiconv.bids import populate_intended_for +from heudiconv.bids import ( + populate_intended_for, + get_shim_setting, + SHIM_KEY, +) import pytest +SHIM_LENGTH = 6 + + +# Test scenarios: +# -file with "ShimSetting" field +# -file with no "ShimSetting", in "foo" dir, should return "foo" +# -file with no "ShimSetting", in "fmap" dir, acq-CatchThis, should return +# "CatchThis" +# -file with no "ShimSetting", in "fmap" dir, acq-fMRI, should return "func" +A_SHIM = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] +@pytest.mark.parametrize( + "fname, content, expected_return", [ + (op.join('foo', 'bar.json'), {SHIM_KEY: A_SHIM}, A_SHIM), + (op.join('dont_catch_this', 'foo', 'bar.json'), {}, 'foo'), + (op.join('dont_catch_this', 'fmap', 'bar_acq-CatchThis.json'), {}, 'CatchThis'), + (op.join('dont_catch_this', 'fmap', 'bar_acq-fMRI.json'), {}, 'func'), + ] +) +def test_get_shim_setting(tmpdir, fname, content, expected_return): + """ Tests for get_shim_setting """ + json_name = op.join(str(tmpdir), fname) + json_dir = op.dirname(json_name) + if not op.exists(json_dir): + os.makedirs(json_dir) + save_json(json_name, content) + assert get_shim_setting(json_name) == expected_return + + # TODO: Do the same with a GRE fmap (magnitude/phase, etc.) +# TODO: Add test for when there are no ShimSettings def create_dummy_pepolar_bids_session(session_path): """ Creates a dummy BIDS session, with slim json files and empty nii.gz + The fmap files are pepolar + The json files have ShimSettings + Parameters: ---------- session_path : str or os.path @@ -22,17 +59,14 @@ def create_dummy_pepolar_bids_session(session_path): """ session_parent, session_basename = op.split(session_path) if session_basename.startswith('ses-'): - subj_folder = session_parent prefix = op.split(session_parent)[1] + '_' + session_basename else: - subj_folder = session_path prefix = session_basename # Generate some random ShimSettings: - shim_length = 6 - dwi_shims = ['{0:.4f}'.format(random()) for i in range(shim_length)] - func_shims_A = ['{0:.4f}'.format(random()) for i in range(shim_length)] - func_shims_B = ['{0:.4f}'.format(random()) for i in range(shim_length)] + dwi_shims = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] + func_shims_A = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] + func_shims_B = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] # Dict with the file structure for the session: # -anat: @@ -58,7 +92,7 @@ def create_dummy_pepolar_bids_session(session_path): '{p}_acq-A_bold.json'.format(p=prefix): {'ShimSetting': func_shims_A}, '{p}_acq-B_bold.json'.format(p=prefix): {'ShimSetting': func_shims_B}, '{p}_acq-unmatched_bold.json'.format(p=prefix): { - 'ShimSetting': ['{0:.4f}'.format(random()) for i in range(shim_length)] + 'ShimSetting': ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] }, }) # -fmap: @@ -140,7 +174,7 @@ def test_populate_intended_for(tmpdir, folder, expected_prefix): ] # runNo=1 goes with acq='A'; runNo=2 goes with acq='B' for runNo, acq in zip([1, 2], ['A', 'B']) - for d in['AP', 'PA'] + for d in ['AP', 'PA'] } ) From 36643ae0475823a34a04c04738e107ebf63c11df Mon Sep 17 00:00:00 2001 From: pvelasco Date: Sat, 19 Dec 2020 15:19:27 -0500 Subject: [PATCH 10/78] RF: Simplify a little bit `create_dummy_pepolar_bids_session` No change in functionality --- heudiconv/tests/test_bids.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py index c547761e..2752a0f8 100644 --- a/heudiconv/tests/test_bids.py +++ b/heudiconv/tests/test_bids.py @@ -71,12 +71,10 @@ def create_dummy_pepolar_bids_session(session_path): # Dict with the file structure for the session: # -anat: anat_struct = { - '{p}_{m}.nii.gz'.format(p=prefix, m=mod): '' for mod in ['T1w', 'T2w'] + '{p}_{m}.{e}'.format(p=prefix, m=mod, e=ext): dummy_content + for ext, dummy_content in zip(['nii.gz', 'json'], ['', {}]) + for mod in ['T1w', 'T2w'] } - anat_struct.update({ - # empty json: - '{p}_{m}.json'.format(p=prefix, m=mod): {} for mod in ['T1w', 'T2w'] - }) # -dwi: dwi_struct = { '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=prefix, r=runNo): '' for runNo in [1, 2] From 1be873ca1ba355a48e36b9546e2122336ffcf0bd Mon Sep 17 00:00:00 2001 From: pvelasco Date: Mon, 21 Dec 2020 14:32:58 -0500 Subject: [PATCH 11/78] RF: `test_bids.py: create_dummy_pepolar_bids_session` now also returns expected IntendedFor This way, when we write future test cases, the test function can be reused as long as it know what the expected IntendedFor are. --- heudiconv/tests/test_bids.py | 77 ++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py index 2752a0f8..f4b86866 100644 --- a/heudiconv/tests/test_bids.py +++ b/heudiconv/tests/test_bids.py @@ -1,4 +1,4 @@ -import json +import re import os import os.path as op from random import random @@ -56,6 +56,14 @@ def create_dummy_pepolar_bids_session(session_path): ---------- session_path : str or os.path path to the session (or subject) level folder + + Returns: + ------- + session_struct : dict + Structure of the directory that was created + expected_result : dict + dictionary with fmap names as keys and the expected "IntendedFor" as + values. """ session_parent, session_basename = op.split(session_path) if session_basename.startswith('ses-'): @@ -63,6 +71,8 @@ def create_dummy_pepolar_bids_session(session_path): else: prefix = session_basename + # 1) Simulate the file structure for a session: + # Generate some random ShimSettings: dwi_shims = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] func_shims_A = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] @@ -119,7 +129,40 @@ def create_dummy_pepolar_bids_session(session_path): } create_tree(session_path, session_struct) - return session_struct + + # 2) Now, let's create a dict with what we expect for the "IntendedFor": + + sub_match = re.findall('(sub-([a-zA-Z0-9]*))', session_path) + sub_str = sub_match[0][0] + expected_prefix = session_path.split(sub_str)[-1].split(op.sep)[-1] + + # dict, with fmap names as keys and the expected "IntendedFor" as values. + expected_result = { + '{p}_acq-dwi_dir-{d}_run-{r}_epi.json'.format(p=prefix, d=d, r=runNo): + intended_for + # (runNo=1 goes with the long list, runNo=2 goes with None): + for runNo, intended_for in zip( + [1, 2], + [[op.join(expected_prefix, 'dwi', '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=prefix, r=r)) for r in [1,2]], + None] + ) + for d in ['AP', 'PA'] + } + expected_result.update( + { + '{p}_acq-fMRI_dir-{d}_run-{r}_epi.json'.format(p=prefix, d=d, r=runNo): + [ + op.join(expected_prefix, + 'func', + '{p}_acq-{a}_bold.nii.gz'.format(p=prefix, a=acq)) + ] + # runNo=1 goes with acq='A'; runNo=2 goes with acq='B' + for runNo, acq in zip([1, 2], ['A', 'B']) + for d in ['AP', 'PA'] + } + ) + + return session_struct, expected_result # Test two scenarios: @@ -146,36 +189,9 @@ def test_populate_intended_for(tmpdir, folder, expected_prefix): """ session_folder = op.join(str(tmpdir), folder) - session_struct = create_dummy_pepolar_bids_session(session_folder) + session_struct, expected_result = create_dummy_pepolar_bids_session(session_folder) populate_intended_for(session_folder) - run_prefix = 'sub-1' + ('_' + expected_prefix if expected_prefix else '') - # dict, with fmap names as keys and the expected "IntendedFor" as values. - expected_result = { - '{p}_acq-dwi_dir-{d}_run-{r}_epi.json'.format(p=run_prefix, d=d, r=runNo): - intended_for - # (runNo=1 goes with the long list, runNo=2 goes with None): - for runNo, intended_for in zip( - [1, 2], - [[op.join(expected_prefix, 'dwi', '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=run_prefix, r=r)) for r in [1,2]], - None] - ) - for d in ['AP', 'PA'] - } - expected_result.update( - { - '{p}_acq-fMRI_dir-{d}_run-{r}_epi.json'.format(p=run_prefix, d=d, r=runNo): - [ - op.join(expected_prefix, - 'func', - '{p}_acq-{a}_bold.nii.gz'.format(p=run_prefix, a=acq)) - ] - # runNo=1 goes with acq='A'; runNo=2 goes with acq='B' - for runNo, acq in zip([1, 2], ['A', 'B']) - for d in ['AP', 'PA'] - } - ) - # Now, loop through the jsons in the fmap folder and make sure it matches # the expected result: fmap_folder = op.join(session_folder, 'fmap') @@ -188,6 +204,7 @@ def test_populate_intended_for(tmpdir, folder, expected_prefix): # Also, make sure the run with random shims is not here: # (It is assured by the assert above, but let's make it # explicit) + run_prefix = j.split('_acq')[0] assert '{p}_acq-unmatched_bold.nii.gz'.format(p=run_prefix) not in data['IntendedFor'] else: assert 'IntendedFor' not in data.keys() From 7815c9244b363012cd2a3cbdcf95b2e711db0d45 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Mon, 21 Dec 2020 15:30:32 -0500 Subject: [PATCH 12/78] Adds tests for the case in which there are no ShimSettings --- heudiconv/tests/test_bids.py | 121 ++++++++++++++++++++++++++++++++--- 1 file changed, 113 insertions(+), 8 deletions(-) diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py index f4b86866..62733d4c 100644 --- a/heudiconv/tests/test_bids.py +++ b/heudiconv/tests/test_bids.py @@ -44,8 +44,6 @@ def test_get_shim_setting(tmpdir, fname, content, expected_return): assert get_shim_setting(json_name) == expected_return -# TODO: Do the same with a GRE fmap (magnitude/phase, etc.) -# TODO: Add test for when there are no ShimSettings def create_dummy_pepolar_bids_session(session_path): """ Creates a dummy BIDS session, with slim json files and empty nii.gz @@ -165,18 +163,123 @@ def create_dummy_pepolar_bids_session(session_path): return session_struct, expected_result -# Test two scenarios: +def create_dummy_no_shim_settings_bids_session(session_path): + """ + Creates a dummy BIDS session, with slim json files and empty nii.gz + The fmap files are pepolar + The json files don't have ShimSettings + + Parameters: + ---------- + session_path : str or os.path + path to the session (or subject) level folder + + Returns: + ------- + session_struct : dict + Structure of the directory that was created + expected_result : dict + dictionary with fmap names as keys and the expected "IntendedFor" as + values. + """ + session_parent, session_basename = op.split(session_path) + if session_basename.startswith('ses-'): + prefix = op.split(session_parent)[1] + '_' + session_basename + else: + prefix = session_basename + + # 1) Simulate the file structure for a session: + + # Dict with the file structure for the session. + # All json files will be empty. + # -anat: + anat_struct = { + '{p}_{m}.{e}'.format(p=prefix, m=mod, e=ext): dummy_content + for ext, dummy_content in zip(['nii.gz', 'json'], ['', {}]) + for mod in ['T1w', 'T2w'] + } + # -dwi: + dwi_struct = { + '{p}_acq-A_run-{r}_dwi.{e}'.format(p=prefix, r=runNo, e=ext): dummy_content + for ext, dummy_content in zip(['nii.gz', 'json'], ['', {}]) + for runNo in [1, 2] + } + # -func: + func_struct = { + '{p}_acq-{a}_bold.{e}'.format(p=prefix, a=acq, e=ext): dummy_content + for ext, dummy_content in zip(['nii.gz', 'json'], ['', {}]) + for acq in ['A', 'B'] + } + # -fmap: + fmap_struct = { + '{p}_acq-{a}_dir-{d}_run-{r}_epi.{e}'.format(p=prefix, a=acq, d=d, r=r, e=ext): dummy_content + for ext, dummy_content in zip(['nii.gz', 'json'], ['', {}]) + for acq in ['dwi', 'fMRI'] + for d in ['AP', 'PA'] + for r in [1, 2] + } + # structure for the full session: + session_struct = { + 'anat': anat_struct, + 'dwi': dwi_struct, + 'func': func_struct, + 'fmap': fmap_struct + } + + create_tree(session_path, session_struct) + + # 2) Now, let's create a dict with what we expect for the "IntendedFor": + + sub_match = re.findall('(sub-([a-zA-Z0-9]*))', session_path) + sub_str = sub_match[0][0] + expected_prefix = session_path.split(sub_str)[-1].split(op.sep)[-1] + + # dict, with fmap names as keys and the expected "IntendedFor" as values. + expected_result = { + # (runNo=1 goes with the long list, runNo=2 goes with None): + '{p}_acq-dwi_dir-{d}_run-{r}_epi.json'.format(p=prefix, d=d, r=runNo): intended_for + for runNo, intended_for in zip( + [1, 2], + [[op.join(expected_prefix, 'dwi', '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=prefix, r=r)) for r in [1,2]], + None] + ) + for d in ['AP', 'PA'] + } + expected_result.update( + { + # The first "fMRI" run gets all files in the "func" folder; + # the second shouldn't get any. + '{p}_acq-fMRI_dir-{d}_run-{r}_epi.json'.format(p=prefix, d=d, r=runNo): intended_for + for runNo, intended_for in zip( + [1, 2], + [[op.join(expected_prefix, 'func', '{p}_acq-{a}_bold.nii.gz'.format(p=prefix, a=acq)) + for acq in ['A', 'B']], + None] + ) + for d in ['AP', 'PA'] + } + ) + + return session_struct, expected_result + + +# Test two scenarios for each case: # -study without sessions # -study with sessions +# Cases: +# A) pepolar fmaps with ShimSetting in json files +# B) same, with no ShimSetting +# TODO: Do the same with a GRE fmap (magnitude/phase, etc.) # The "expected_prefix" (the beginning of the path to the "IntendedFor") # should be relative to the subject level @pytest.mark.parametrize( - "folder, expected_prefix", [ - ('no_sessions/sub-1', ''), - ('sessions/sub-1/ses-pre', 'ses-pre') + "folder, expected_prefix, simulation_function", [ + (folder, expected_prefix, sim_func) + for folder, expected_prefix in zip(['no_sessions/sub-1', 'sessions/sub-1/ses-pre'], ['', 'ses-pre']) + for sim_func in [create_dummy_pepolar_bids_session, create_dummy_no_shim_settings_bids_session] ] ) -def test_populate_intended_for(tmpdir, folder, expected_prefix): +def test_populate_intended_for(tmpdir, folder, expected_prefix, simulation_function): """ Test populate_intended_for. Parameters: @@ -186,10 +289,12 @@ def test_populate_intended_for(tmpdir, folder, expected_prefix): path to BIDS study to be simulated, relative to tmpdir expected_prefix : str expected start of the "IntendedFor" elements + simulation_function : function + function to create the directory tree and expected results """ session_folder = op.join(str(tmpdir), folder) - session_struct, expected_result = create_dummy_pepolar_bids_session(session_folder) + session_struct, expected_result = simulation_function(session_folder) populate_intended_for(session_folder) # Now, loop through the jsons in the fmap folder and make sure it matches From 4b21d457772403ce50aed040ae1b12c3871c4f93 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Tue, 22 Dec 2020 13:51:09 -0500 Subject: [PATCH 13/78] BF: Fixes bug in `populate_intended_for` Removing an element from a (sorted) list while looping through the list was not a good idea, and it failed in some cases. Now we keep a parallel list with elements already accounted for and, while looping through the list, we only act if the element is not already accounted for. --- heudiconv/bids.py | 69 ++++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 42e430d2..373d477c 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -566,37 +566,40 @@ def populate_intended_for(path_to_bids_session): # same IntendedFor list to those other corresponding fmap json files, and # remove them from the list of available fmap json files. # Once we have gone through all the fmap json files, we are done. - jsons_accounted_for = set() + runs_accounted_for = set() + fmaps_accounted_for = set() for fm_json in fmap_jsons: - lgr.debug('Looking for runs for {}'.format(fm_json)) - fm_shims = get_shim_setting(fm_json) - - intended_for = [] - for image_json in session_jsons: - image_shims = get_shim_setting(image_json) - if image_shims == fm_shims: - # BIDS specifies that the intended for are: - # - **image** files - # - path relative to the **subject level** - image_json_relative_path = op.relpath(image_json, start=bids_folder) - # image_json_relative_path[:-5] removes the '.json' extension: - intended_for.append( - image_json_relative_path[:-5] + '.nii.gz' - ) - jsons_accounted_for.add(image_json) - if len(intended_for) > 0: - fm_json_name = op.basename(fm_json) - # get and labels: - acq_match = re.findall('([/_]acq-([a-zA-Z0-9]*))', fm_json_name) - acq_str = acq_match[0][0] if acq_match else '' - run_match = re.findall('([/_]run-([a-zA-Z0-9]*))', fm_json_name) - run_str = run_match[0][0] if run_match else '' - # Loop through all the files that have the same "acq-" and "run-" - intended_for = sorted([str(f) for f in intended_for]) - for other_fm_json in glob(op.join(path_to_bids_session, 'fmap/*' + acq_str + '*' + run_str + '*.json')): - # add the IntendedFor field to the json file: - add_field_to_json(other_fm_json, {"IntendedFor": intended_for}) - fmap_jsons.remove(other_fm_json) - # Remove the runs accounted for from the session_jsons list, so that - # we don't assign another fmap to this image: - session_jsons -= jsons_accounted_for + if fm_json not in fmaps_accounted_for: + lgr.debug('Looking for runs for {}'.format(fm_json)) + fm_shims = get_shim_setting(fm_json) + + intended_for = [] + for image_json in session_jsons: + image_shims = get_shim_setting(image_json) + if image_shims == fm_shims: + # BIDS specifies that the intended for are: + # - **image** files + # - path relative to the **subject level** + image_json_relative_path = op.relpath(image_json, start=bids_folder) + # image_json_relative_path[:-5] removes the '.json' extension: + intended_for.append( + image_json_relative_path[:-5] + '.nii.gz' + ) + runs_accounted_for.add(image_json) + if len(intended_for) > 0: + intended_for = sorted([str(f) for f in intended_for]) + # find all fmap json files with the same and entities: + fm_json_name = op.basename(fm_json) + acq_match = re.findall('([/_]acq-([a-zA-Z0-9]*))', fm_json_name) + acq_str = acq_match[0][0] if acq_match else '' + run_match = re.findall('([/_]run-([a-zA-Z0-9]*))', fm_json_name) + run_str = run_match[0][0] if run_match else '' + # Loop through all the files that have the same "acq-" and "run-" + # Note: the following loop will also include 'fm_json' + for linked_fm_json in glob(op.join(path_to_bids_session, 'fmap/*' + acq_str + '*' + run_str + '*.json')): + # add the IntendedFor field to the json file: + add_field_to_json(linked_fm_json, {"IntendedFor": intended_for}) + fmaps_accounted_for.update({linked_fm_json}) + # Remove the runs accounted for from the session_jsons list, so that + # we don't assign another fmap to this image: + session_jsons -= runs_accounted_for From 03d40050d2c45ec267af1a05f01b1ce316a45ce5 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Tue, 22 Dec 2020 13:53:41 -0500 Subject: [PATCH 14/78] Adds a new test case for `populate_intended_for` It coves the cases in which the fmaps are in the form of magnitude/phase maps --- heudiconv/tests/test_bids.py | 125 +++++++++++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 4 deletions(-) diff --git a/heudiconv/tests/test_bids.py b/heudiconv/tests/test_bids.py index 62733d4c..82887b26 100644 --- a/heudiconv/tests/test_bids.py +++ b/heudiconv/tests/test_bids.py @@ -229,6 +229,9 @@ def create_dummy_no_shim_settings_bids_session(session_path): create_tree(session_path, session_struct) # 2) Now, let's create a dict with what we expect for the "IntendedFor": + # NOTE: The "expected_prefix" (the beginning of the path to the + # "IntendedFor") should be relative to the subject level (see: + # https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#fieldmap-data) sub_match = re.findall('(sub-([a-zA-Z0-9]*))', session_path) sub_str = sub_match[0][0] @@ -263,20 +266,134 @@ def create_dummy_no_shim_settings_bids_session(session_path): return session_struct, expected_result +def create_dummy_magnitude_phase_bids_session(session_path): + """ + Creates a dummy BIDS session, with slim json files and empty nii.gz + The fmap files are a magnitude/phase pair + The json files have ShimSettings + We just need to test a very simple case to make sure the mag/phase have + the same "IntendedFor" field: + + Parameters: + ---------- + session_path : str or os.path + path to the session (or subject) level folder + + Returns: + ------- + session_struct : dict + Structure of the directory that was created + expected_result : dict + dictionary with fmap names as keys and the expected "IntendedFor" as + values. + """ + session_parent, session_basename = op.split(session_path) + if session_basename.startswith('ses-'): + prefix = op.split(session_parent)[1] + '_' + session_basename + else: + prefix = session_basename + + # 1) Simulate the file structure for a session: + + # Generate some random ShimSettings: + dwi_shims = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] + func_shims_A = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] + func_shims_B = ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] + + # Dict with the file structure for the session: + # -dwi: + dwi_struct = { + '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=prefix, r=runNo): '' for runNo in [1, 2] + } + dwi_struct.update({ + '{p}_acq-A_run-{r}_dwi.json'.format(p=prefix, r=runNo): {'ShimSetting': dwi_shims} for runNo in [1, 2] + }) + # -func: + func_struct = { + '{p}_acq-{a}_bold.nii.gz'.format(p=prefix, a=acq): '' for acq in ['A', 'B', 'unmatched'] + } + func_struct.update({ + '{p}_acq-A_bold.json'.format(p=prefix): {'ShimSetting': func_shims_A}, + '{p}_acq-B_bold.json'.format(p=prefix): {'ShimSetting': func_shims_B}, + '{p}_acq-unmatched_bold.json'.format(p=prefix): { + 'ShimSetting': ['{0:.4f}'.format(random()) for i in range(SHIM_LENGTH)] + }, + }) + # -fmap: + # * Case 1 in https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#fieldmap-data + fmap_struct = { + '{p}_acq-case1_{s}.nii.gz'.format(p=prefix, s=suffix): '' + for suffix in ['phasediff', 'magnitude1', 'magnitude2'] + } + fmap_struct.update({ + '{p}_acq-case1_{s}.json'.format(p=prefix, s='phasediff'): {'ShimSetting': dwi_shims} + }) + # * Case 2: + fmap_struct.update({ + '{p}_acq-case2_{s}.nii.gz'.format(p=prefix, s=suffix): '' + for suffix in ['magnitude1', 'magnitude2', 'phase1', 'phase2'] + }) + fmap_struct.update({ + '{p}_acq-case2_phase{n}.json'.format(p=prefix, n=n): {'ShimSetting': func_shims_A} + for n in [1, 2] + }) + # * Case 3: + fmap_struct.update({ + '{p}_acq-case3_{s}.nii.gz'.format(p=prefix, s=suffix): '' + for suffix in ['magnitude', 'fieldmap'] + }) + fmap_struct.update({ + '{p}_acq-case3_fieldmap.json'.format(p=prefix): {'ShimSetting': func_shims_B} + }) + # structure for the full session: + session_struct = { + 'dwi': dwi_struct, + 'func': func_struct, + 'fmap': fmap_struct + } + + create_tree(session_path, session_struct) + + # 2) Now, let's create a dict with what we expect for the "IntendedFor": + + sub_match = re.findall('(sub-([a-zA-Z0-9]*))', session_path) + sub_str = sub_match[0][0] + expected_prefix = session_path.split(sub_str)[-1].split(op.sep)[-1] + + # dict, with fmap names as keys and the expected "IntendedFor" as values. + expected_result = { + '{p}_acq-case1_{s}.json'.format(p=prefix, s='phasediff'): + [op.join(expected_prefix, 'dwi', '{p}_acq-A_run-{r}_dwi.nii.gz'.format(p=prefix, r=r)) for r in [1, 2]] + } + expected_result.update({ + '{p}_acq-case2_phase{n}.json'.format(p=prefix, n=n): + # populate_intended_for writes lists: + [op.join(expected_prefix, 'func', '{p}_acq-A_bold.nii.gz'.format(p=prefix))] + for n in [1, 2] + }) + expected_result.update({ + '{p}_acq-case3_fieldmap.json'.format(p=prefix): + # populate_intended_for writes lists: + [op.join(expected_prefix, 'func', '{p}_acq-B_bold.nii.gz'.format(p=prefix))] + }) + + return session_struct, expected_result + + # Test two scenarios for each case: # -study without sessions # -study with sessions # Cases: # A) pepolar fmaps with ShimSetting in json files # B) same, with no ShimSetting -# TODO: Do the same with a GRE fmap (magnitude/phase, etc.) -# The "expected_prefix" (the beginning of the path to the "IntendedFor") -# should be relative to the subject level +# C) magnitude/phase, with ShimSetting @pytest.mark.parametrize( "folder, expected_prefix, simulation_function", [ (folder, expected_prefix, sim_func) for folder, expected_prefix in zip(['no_sessions/sub-1', 'sessions/sub-1/ses-pre'], ['', 'ses-pre']) - for sim_func in [create_dummy_pepolar_bids_session, create_dummy_no_shim_settings_bids_session] + for sim_func in [create_dummy_pepolar_bids_session, + create_dummy_no_shim_settings_bids_session, + create_dummy_magnitude_phase_bids_session] ] ) def test_populate_intended_for(tmpdir, folder, expected_prefix, simulation_function): From d3c9962e7eea6963f699cc3e20aa68c110e3e286 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Wed, 23 Dec 2020 16:06:41 -0500 Subject: [PATCH 15/78] RF: `test_convert.py` imports full module `convert.py` To prepare for a future `test_convert.test_convert`, which requires mocking some functions --- heudiconv/tests/test_convert.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/heudiconv/tests/test_convert.py b/heudiconv/tests/test_convert.py index 75c2194a..e6ee0d51 100644 --- a/heudiconv/tests/test_convert.py +++ b/heudiconv/tests/test_convert.py @@ -2,9 +2,7 @@ """ import pytest -from heudiconv.convert import (update_complex_name, - update_multiecho_name, - update_uncombined_name) +from heudiconv import convert from heudiconv.bids import BIDSError @@ -17,17 +15,17 @@ def test_update_complex_name(): metadata = {'ImageType': ['ORIGINAL', 'PRIMARY', 'P', 'MB', 'TE3', 'ND', 'MOSAIC']} suffix = 3 out_fn_true = 'sub-X_ses-Y_task-Z_run-01_part-phase_sbref' - out_fn_test = update_complex_name(metadata, fn, suffix) + out_fn_test = convert.update_complex_name(metadata, fn, suffix) assert out_fn_test == out_fn_true # Catch an unsupported type and *do not* update fn = 'sub-X_ses-Y_task-Z_run-01_phase' - out_fn_test = update_complex_name(metadata, fn, suffix) + out_fn_test = convert.update_complex_name(metadata, fn, suffix) assert out_fn_test == fn # Data type is missing from metadata so use suffix fn = 'sub-X_ses-Y_task-Z_run-01_sbref' metadata = {'ImageType': ['ORIGINAL', 'PRIMARY', 'MB', 'TE3', 'ND', 'MOSAIC']} out_fn_true = 'sub-X_ses-Y_task-Z_run-01_part-3_sbref' - out_fn_test = update_complex_name(metadata, fn, suffix) + out_fn_test = convert.update_complex_name(metadata, fn, suffix) assert out_fn_test == out_fn_true # Catch existing field with value that *does not match* metadata # and raise Exception @@ -35,7 +33,7 @@ def test_update_complex_name(): metadata = {'ImageType': ['ORIGINAL', 'PRIMARY', 'P', 'MB', 'TE3', 'ND', 'MOSAIC']} suffix = 3 with pytest.raises(BIDSError): - assert update_complex_name(metadata, fn, suffix) + assert convert.update_complex_name(metadata, fn, suffix) def test_update_multiecho_name(): @@ -48,15 +46,15 @@ def test_update_multiecho_name(): 'EchoNumber': 1} echo_times = [0.01, 0.02, 0.03] out_fn_true = 'sub-X_ses-Y_task-Z_run-01_echo-1_bold' - out_fn_test = update_multiecho_name(metadata, fn, echo_times) + out_fn_test = convert.update_multiecho_name(metadata, fn, echo_times) assert out_fn_test == out_fn_true # EchoNumber field is missing from metadata, so use echo_times metadata = {'EchoTime': 0.01} - out_fn_test = update_multiecho_name(metadata, fn, echo_times) + out_fn_test = convert.update_multiecho_name(metadata, fn, echo_times) assert out_fn_test == out_fn_true # Catch an unsupported type and *do not* update fn = 'sub-X_ses-Y_task-Z_run-01_phasediff' - out_fn_test = update_multiecho_name(metadata, fn, echo_times) + out_fn_test = convert.update_multiecho_name(metadata, fn, echo_times) assert out_fn_test == fn @@ -69,10 +67,10 @@ def test_update_uncombined_name(): metadata = {'CoilString': 'H1'} channel_names = ['H1', 'H2', 'H3', 'HEA;HEP'] out_fn_true = 'sub-X_ses-Y_task-Z_run-01_ch-01_bold' - out_fn_test = update_uncombined_name(metadata, fn, channel_names) + out_fn_test = convert.update_uncombined_name(metadata, fn, channel_names) assert out_fn_test == out_fn_true # CoilString field has no number in it metadata = {'CoilString': 'HEA;HEP'} out_fn_true = 'sub-X_ses-Y_task-Z_run-01_ch-04_bold' - out_fn_test = update_uncombined_name(metadata, fn, channel_names) + out_fn_test = convert.update_uncombined_name(metadata, fn, channel_names) assert out_fn_test == out_fn_true From 1bd783aab49fda5bac90d58bd071f9c47ae20a89 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Wed, 23 Dec 2020 16:14:21 -0500 Subject: [PATCH 16/78] ENH: Adds a call to `populate_intended_for` in `convert.py:convert` It also adds the corresponding unit tests. --- heudiconv/convert.py | 17 +++++++++++ heudiconv/tests/test_convert.py | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 1f6a4136..92d6bd4b 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -26,6 +26,7 @@ from .bids import ( convert_sid_bids, populate_bids_templates, + populate_intended_for, save_scans_key, tuneup_bids_json_files, add_participant_record, @@ -519,6 +520,22 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov, if custom_callable is not None: custom_callable(*item) + # Populate "IntendedFor" for fmap files. + # Because fmap files can only be used to correct for distortions in images + # collected within the same scanning session, find unique subject/session + # combinations from the outname in each item: + outnames = [item[0] for item in items] + # - grab "sub-[/ses-]", and keep only unique ones: + sessions = set( + re.search( + 'sub-(?P[a-zA-Z0-9]*)([{0}_]ses-(?P[a-zA-Z0-9]*))?'.format(op.sep), + oname + ).group(0) + for oname in outnames + ) + for ses in sessions: + populate_intended_for(ses) + def convert_dicom(item_dicoms, bids_options, prefix, outdir, tempdirs, symlink, overwrite): diff --git a/heudiconv/tests/test_convert.py b/heudiconv/tests/test_convert.py index e6ee0d51..63d0f0ed 100644 --- a/heudiconv/tests/test_convert.py +++ b/heudiconv/tests/test_convert.py @@ -1,5 +1,6 @@ """Test functions in heudiconv.convert module. """ +from os import path as op import pytest from heudiconv import convert @@ -74,3 +75,54 @@ def test_update_uncombined_name(): out_fn_true = 'sub-X_ses-Y_task-Z_run-01_ch-04_bold' out_fn_test = convert.update_uncombined_name(metadata, fn, channel_names) assert out_fn_test == out_fn_true + + +# Test two scenarios for each case: +# -study without sessions +# -study with sessions +@pytest.mark.parametrize( + "subjects, sesID, expected_session_folder", [ + (['Jason', 'Bourne'], None, 'sub-{sID}'), + ('Bourne', 'Treadstone', 'sub-{{sID}}{sep}ses-{{ses}}'.format(sep=op.sep)), + ] +) +def test_convert(monkeypatch, capfd, + subjects, sesID, expected_session_folder): + """ + Test convert + + For now, I'm just going to test that the call to populate_intended_for is + done with the correct argument. + More tests can be added here. + """ + + def mock_populate_intended_for(session): + """ + Pretend we run populate_intended_for, but just print out the argument. + """ + print('session: {}'.format(session)) + return + monkeypatch.setattr(convert, "populate_intended_for", mock_populate_intended_for) + + outdir = op.sep + 'foo' + outfolder = op.join(outdir, 'sub-{sID}', 'ses-{ses}' if sesID else '') + sub_ses = 'sub-{sID}' + ('_ses-{ses}' if sesID else '') + + # items are a list of tuples, with each tuple having three elements: + # prefix, outtypes, item_dicoms + items = [ + (op.join(outfolder, 'anat', sub_ses + '_T1w').format(sID=s, ses=sesID), ('',), []) + for s in subjects + ] + + convert.convert(items, + converter='', + scaninfo_suffix='.json', + custom_callable=None, + with_prov=None, + bids_options=[], + outdir=outdir, + min_meta=True, + overwrite=False) + output = capfd.readouterr() + assert ['session: sub-{}'.format(s) in output.out for s in subjects] From d9dabe73d78dadade993898cae3473575a35efe8 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Mon, 4 Jan 2021 14:17:03 -0500 Subject: [PATCH 17/78] BF: Fixes unit tests for test_convert.py We make sure we don't try to write to the root directory --- heudiconv/tests/test_convert.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/heudiconv/tests/test_convert.py b/heudiconv/tests/test_convert.py index 63d0f0ed..192985a6 100644 --- a/heudiconv/tests/test_convert.py +++ b/heudiconv/tests/test_convert.py @@ -86,7 +86,7 @@ def test_update_uncombined_name(): ('Bourne', 'Treadstone', 'sub-{{sID}}{sep}ses-{{ses}}'.format(sep=op.sep)), ] ) -def test_convert(monkeypatch, capfd, +def test_convert(tmpdir, monkeypatch, capfd, subjects, sesID, expected_session_folder): """ Test convert @@ -104,7 +104,7 @@ def mock_populate_intended_for(session): return monkeypatch.setattr(convert, "populate_intended_for", mock_populate_intended_for) - outdir = op.sep + 'foo' + outdir = op.join(tmpdir, 'foo') outfolder = op.join(outdir, 'sub-{sID}', 'ses-{ses}' if sesID else '') sub_ses = 'sub-{sID}' + ('_ses-{ses}' if sesID else '') From fb7dd700209a9e890c461bc8d71a25c3ae2b51eb Mon Sep 17 00:00:00 2001 From: pvelasco Date: Mon, 4 Jan 2021 15:06:09 -0500 Subject: [PATCH 18/78] BF: Fixes unit tests for test_convert.py --- heudiconv/tests/test_convert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/tests/test_convert.py b/heudiconv/tests/test_convert.py index 192985a6..0b2ddd28 100644 --- a/heudiconv/tests/test_convert.py +++ b/heudiconv/tests/test_convert.py @@ -104,7 +104,7 @@ def mock_populate_intended_for(session): return monkeypatch.setattr(convert, "populate_intended_for", mock_populate_intended_for) - outdir = op.join(tmpdir, 'foo') + outdir = op.join(str(tmpdir), 'foo') outfolder = op.join(outdir, 'sub-{sID}', 'ses-{ses}' if sesID else '') sub_ses = 'sub-{sID}' + ('_ses-{ses}' if sesID else '') From 015f79c62559f392de05fcc0f641e3a899f64227 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Tue, 5 Jan 2021 15:07:58 -0500 Subject: [PATCH 19/78] BF: skips populate_intended_for for non-BIDS-compliant datasets --- heudiconv/convert.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 92d6bd4b..17c3ba77 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -526,13 +526,19 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov, # combinations from the outname in each item: outnames = [item[0] for item in items] # - grab "sub-[/ses-]", and keep only unique ones: - sessions = set( - re.search( - 'sub-(?P[a-zA-Z0-9]*)([{0}_]ses-(?P[a-zA-Z0-9]*))?'.format(op.sep), - oname - ).group(0) - for oname in outnames - ) + try: + sessions = set( + re.search( + 'sub-(?P[a-zA-Z0-9]*)([{0}_]ses-(?P[a-zA-Z0-9]*))?'.format(op.sep), + oname + ).group(0) + for oname in outnames + ) + except AttributeError: + # "sub-[/ses-]" is not present, so this is not BIDS compliant + # and it doesn't make sense to add "IntendedFor": + sessions = set() + for ses in sessions: populate_intended_for(ses) From 0690c1aea85fef7c99a2e11885b6bd067bc58c28 Mon Sep 17 00:00:00 2001 From: Pablo Velasco Date: Thu, 18 Feb 2021 15:42:57 -0500 Subject: [PATCH 20/78] Update heudiconv/bids.py Co-authored-by: Yaroslav Halchenko --- heudiconv/bids.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 373d477c..11d826eb 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -570,7 +570,7 @@ def populate_intended_for(path_to_bids_session): fmaps_accounted_for = set() for fm_json in fmap_jsons: if fm_json not in fmaps_accounted_for: - lgr.debug('Looking for runs for {}'.format(fm_json)) + lgr.debug('Looking for runs for %s', fm_json) fm_shims = get_shim_setting(fm_json) intended_for = [] From 9b397de36544e297b88cf27983bcbfb05f0d7b9a Mon Sep 17 00:00:00 2001 From: pvelasco Date: Thu, 18 Feb 2021 15:55:42 -0500 Subject: [PATCH 21/78] Improve lgr in `bids.py` --- heudiconv/bids.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 11d826eb..fb726ad6 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -522,7 +522,6 @@ def populate_intended_for(path_to_bids_session): path to the session folder (or to the subject folder, if there are no sessions). """ - lgr.info('') lgr.info('Adding "IntendedFor" to the fieldmaps in {}.'.format(path_to_bids_session)) # Resolve path (eliminate '..') @@ -536,8 +535,7 @@ def populate_intended_for(path_to_bids_session): fmap_dir = op.join(path_to_bids_session, 'fmap') if not op.exists(fmap_dir): - lgr.warning('Fmap folder not found in {}.'.format(path_to_bids_session)) - lgr.warning('We cannot add the IntendedFor field') + lgr.warning('We cannot add the IntendedFor field: no fmap/ in %s', path_to_bids_session) return # Get a list of all fmap json files in the session: From d801feecd612342faa7051166cf98e962e825137 Mon Sep 17 00:00:00 2001 From: Pablo Velasco Date: Fri, 19 Feb 2021 13:37:28 -0500 Subject: [PATCH 22/78] Simplify sorting of fmaps in heudiconv/bids.py Co-authored-by: Yaroslav Halchenko --- heudiconv/bids.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 8ae302ee..265005e5 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -540,7 +540,7 @@ def populate_intended_for(path_to_bids_session): # Get a list of all fmap json files in the session: # (we will remove elements later on, so don't just iterate) - fmap_jsons = sorted([j for j in glob(op.join(path_to_bids_session, 'fmap/*.json'))]) + fmap_jsons = sorted(glob(op.join(path_to_bids_session, 'fmap/*.json'))) # Get a set with all non-fmap json files in the session (set is easier): # We also exclude the SBRef files. From 8f25fb666ad34a062e28e2566f92c35a87c42fc7 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Tue, 2 Mar 2021 09:19:10 -0500 Subject: [PATCH 23/78] Simplify expression to find session field maps --- heudiconv/bids.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 8ae302ee..4ff9c44f 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -539,8 +539,8 @@ def populate_intended_for(path_to_bids_session): return # Get a list of all fmap json files in the session: - # (we will remove elements later on, so don't just iterate) - fmap_jsons = sorted([j for j in glob(op.join(path_to_bids_session, 'fmap/*.json'))]) + # (we will remove elements later on, so don't just get an iterator) + fmap_jsons = sorted(glob(op.join(path_to_bids_session, 'fmap/*.json'))) # Get a set with all non-fmap json files in the session (set is easier): # We also exclude the SBRef files. From b9a1d7a5b4a1ecfde171d6243b93bfcbaba4da71 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Tue, 2 Mar 2021 09:51:30 -0500 Subject: [PATCH 24/78] Minor: Use preferred str formatting in lgr call in bids.py --- heudiconv/bids.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 4ff9c44f..ad73ea8f 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -522,7 +522,7 @@ def populate_intended_for(path_to_bids_session): path to the session folder (or to the subject folder, if there are no sessions). """ - lgr.info('Adding "IntendedFor" to the fieldmaps in {}.'.format(path_to_bids_session)) + lgr.info('Adding "IntendedFor" to the fieldmaps in %s.', path_to_bids_session) # Resolve path (eliminate '..') path_to_bids_session = op.abspath(path_to_bids_session) From 37c9b7a9316d9dacfd66959449ecf3ff3c0aec84 Mon Sep 17 00:00:00 2001 From: pvelasco Date: Wed, 10 Mar 2021 15:14:04 -0500 Subject: [PATCH 25/78] RF: Add find_fmap_groups for populating IntendedFor --- heudiconv/bids.py | 85 ++++++++++++++++++++++++++---------- heudiconv/tests/test_bids.py | 50 +++++++++++++++++++-- 2 files changed, 109 insertions(+), 26 deletions(-) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index ad73ea8f..fe4235f3 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -493,6 +493,52 @@ def get_shim_setting(json_file): return shims +def find_fmap_groups(fmap_dir): + """ + Finds the different fmap groups in a fmap directory. + By groups here we mean fmaps that are intended to go together + (with reversed PE polarity, magnitude/phase, etc.) + + Parameters: + ---------- + fmap_dir : str or os.path + path to the session folder (or to the subject folder, if there are no + sessions). + + Returns: + ------- + fmap_groups : dict + key: prefix common to the group + value: list of all fmap paths in the group + """ + if op.basename(fmap_dir) != 'fmap': + lgr.error('%s is not a fieldmap folder', fmap_dir) + + # Get a list of all fmap json files in the session: + fmap_jsons = sorted(glob(op.join(fmap_dir, '*.json'))) + + # RegEx to remove fmap-specific substrings from fmap file names + # "_phase[1,2]", "_magnitude[1,2]", "_phasediff", "_dir-