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
23 changes: 19 additions & 4 deletions fmriprep/cli/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@
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}'

Check warning on line 70 in fmriprep/cli/parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/parser.py#L69-L70

Added lines #L69 - L70 were not covered by tests
else:
k, v = kv.split('=')
d[k] = Path(v)

Check warning on line 73 in fmriprep/cli/parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/parser.py#L72-L73

Added lines #L72 - L73 were not covered by tests
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 +233,15 @@
g_bids.add_argument(
'-d',
'--derivatives',
action='store',
metavar='PATH',
type=Path,
action=ToDict,
metavar='PACKAGE=PATH',
type=str,
nargs='*',
tsalo marked this conversation as resolved.
Show resolved Hide resolved
help='Search PATH(s) for pre-computed derivatives.',
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 @@
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()

Check warning on line 246 in fmriprep/cli/tests/test_parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/tests/test_parser.py#L246

Added line #L246 was not covered by tests

# 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()

Check warning on line 252 in fmriprep/cli/tests/test_parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/tests/test_parser.py#L249-L252

Added lines #L249 - L252 were not covered by tests

# Providing --derivatives with names should use them
temp_args = args + [

Check warning on line 255 in fmriprep/cli/tests/test_parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/tests/test_parser.py#L255

Added line #L255 was not covered by tests
'--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()

Check warning on line 261 in fmriprep/cli/tests/test_parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/tests/test_parser.py#L259-L261

Added lines #L259 - L261 were not covered by tests
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 @@
]

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

Check warning on line 119 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L119

Added line #L119 was not covered by tests

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 @@

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 @@
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

Check warning on line 530 in fmriprep/config.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/config.py#L530

Added line #L530 was not covered by tests
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 @@

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():

Check warning on line 252 in fmriprep/workflows/base.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/workflows/base.py#L252

Added line #L252 was not covered by tests
anatomical_cache.update(
collect_anat_derivatives(
derivatives_dir=deriv_dir,
Expand Down Expand Up @@ -653,7 +653,7 @@

entities = extract_entities(bold_series)

for deriv_dir in config.execution.derivatives:
for deriv_dir in config.execution.derivatives.values():

Check warning on line 656 in fmriprep/workflows/base.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/workflows/base.py#L656

Added line #L656 was not covered by tests
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