Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Support named derivative paths #3264

Merged
merged 13 commits into from
Mar 29, 2024
26 changes: 21 additions & 5 deletions fmriprep/cli/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ def __call__(self, parser, namespace, values, option_string=None):
print(msg, file=sys.stderr)
delattr(namespace, self.dest)

class ToDict(Action):
def __call__(self, parser, namespace, values, option_string=None):
d = {}
for i_kv, kv in enumerate(values):
if '=' not in kv:
k = f'deriv-{i_kv}'
v = kv
else:
k, v = kv.split('=')
d[k] = Path(v)
tsalo marked this conversation as resolved.
Show resolved Hide resolved
setattr(namespace, self.dest, d)

def _path_exists(path, parser):
"""Ensure a given path exists."""
if path is None or not Path(path).exists():
Expand Down Expand Up @@ -222,11 +234,15 @@ def _slice_time_ref(value, parser):
g_bids.add_argument(
'-d',
'--derivatives',
action='store',
metavar='PATH',
type=Path,
nargs='*',
help='Search PATH(s) for pre-computed derivatives.',
action=ToDict,
metavar='PACKAGE=PATH',
type=str,
nargs='+',
help=(
'Search PATH(s) for pre-computed derivatives. '
'These may be provided as named folders '
'(e.g., `--derivatives smriprep=/path/to/smriprep`).'
),
)
g_bids.add_argument(
'--bids-database-dir',
Expand Down
31 changes: 31 additions & 0 deletions fmriprep/cli/tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,34 @@ def test_use_syn_sdc(tmp_path, args, expectation):
assert opts.use_syn_sdc == expectation

_reset_config()


def test_derivatives(tmp_path):
"""Check the correct parsing of the derivatives argument."""
bids_path = tmp_path / 'data'
out_path = tmp_path / 'out'
args = [str(bids_path), str(out_path), 'participant']
bids_path.mkdir()

parser = _build_parser()

# Providing --derivatives without a path should raise an error
temp_args = args + ['--derivatives']
with pytest.raises((SystemExit, ArgumentError)):
parser.parse_args(temp_args)
_reset_config()

# Providing --derivatives without names should automatically label them
temp_args = args + ['--derivatives', str(bids_path / 'derivatives/smriprep')]
opts = parser.parse_args(temp_args)
assert opts.derivatives == {'deriv-0': str(bids_path / 'derivatives/smriprep')}
_reset_config()

# Providing --derivatives with names should use them
temp_args = args + [
'--derivatives',
f'smriprep={str(bids_path / "derivatives/smriprep")}',
]
opts = parser.parse_args(temp_args)
assert opts.derivatives == {'smriprep': str(bids_path / 'derivatives/smriprep')}
_reset_config()
2 changes: 1 addition & 1 deletion fmriprep/cli/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def build_workflow(config_file, retval):
]

if config.execution.derivatives:
init_msg += [f'Searching for derivatives: {config.execution.derivatives}.']
init_msg += [f'Searching for derivatives: {list(config.execution.derivatives.values())}.']

if config.execution.fs_subjects_dir:
init_msg += [f"Pre-run FreeSurfer's SUBJECTS_DIR: {config.execution.fs_subjects_dir}."]
Expand Down
6 changes: 3 additions & 3 deletions fmriprep/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ class execution(_Config):

bids_dir = None
"""An existing path to the dataset, which must be BIDS-compliant."""
derivatives = []
derivatives = {}
"""Path(s) to search for pre-computed derivatives"""
bids_database_dir = None
"""Path to the directory containing SQLite database indices for the input BIDS dataset."""
Expand Down Expand Up @@ -526,8 +526,8 @@ def _process_value(value):
cls.bids_filters[acq][k] = _process_value(v)

dataset_links = {'raw': cls.bids_dir}
for i_deriv, deriv_path in enumerate(cls.derivatives):
dataset_links[f'deriv-{i_deriv}'] = deriv_path
for deriv_name, deriv_path in cls.derivatives.items():
dataset_links[deriv_name] = deriv_path
cls.dataset_links = dataset_links

if 'all' in cls.debug:
Expand Down
4 changes: 2 additions & 2 deletions fmriprep/workflows/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ def init_single_subject_wf(subject_id: str):

std_spaces = spaces.get_spaces(nonstandard=False, dim=(3,))
std_spaces.append('fsnative')
for deriv_dir in config.execution.derivatives:
for deriv_dir in config.execution.derivatives.values():
anatomical_cache.update(
collect_anat_derivatives(
derivatives_dir=deriv_dir,
Expand Down Expand Up @@ -653,7 +653,7 @@ def init_single_subject_wf(subject_id: str):

entities = extract_entities(bold_series)

for deriv_dir in config.execution.derivatives:
for deriv_dir in config.execution.derivatives.values():
functional_cache.update(
collect_derivatives(
derivatives_dir=deriv_dir,
Expand Down
2 changes: 1 addition & 1 deletion wrapper/src/fmriprep_docker/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ def main():
unknown_args.append('--derivatives')
for deriv, deriv_path in opts.derivatives.items():
command.extend(['-v', '{}:/deriv/{}:ro'.format(deriv_path, deriv)])
unknown_args.append('/deriv/{}'.format(deriv))
unknown_args.append('{}=/deriv/{}'.format(deriv, deriv))
tsalo marked this conversation as resolved.
Show resolved Hide resolved

if opts.work_dir:
command.extend(['-v', ':'.join((opts.work_dir, '/scratch'))])
Expand Down
Loading