Skip to content

Commit

Permalink
ENH: Support named derivative paths (#3264)
Browse files Browse the repository at this point in the history
Closes #3260.

## Changes proposed in this pull request

- Copy over `ToDict` action from fmriprep-docker over to the main CLI
parser.
- Modify the Config to handle named derivative paths.
- E.g., `--derivatives anat=/path/to/smriprep` --> `DatasetLinks =
{'anat': '/path/to/smriprep'}`
- Infer derivative name from the folder name of the derivative path if
it isn't labeled.
- E.g., `--derivatives /path/to/smriprep` --> `DatasetLinks =
{'smriprep': '/path/to/smriprep'}`

---------

Co-authored-by: Chris Markiewicz <[email protected]>
  • Loading branch information
tsalo and effigies authored Mar 29, 2024
1 parent 8eb8215 commit ecf8580
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 11 deletions.
31 changes: 26 additions & 5 deletions fmriprep/cli/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,23 @@ 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 spec in values:
try:
name, loc = spec.split('=')
loc = Path(loc)
except ValueError:
loc = Path(spec)
name = loc.name

if name in d:
raise ValueError(f'Received duplicate derivative name: {name}')

d[name] = loc
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 +239,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
42 changes: 42 additions & 0 deletions fmriprep/cli/tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,45 @@ 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 == {'smriprep': bids_path / 'derivatives/smriprep'}
_reset_config()

# Providing --derivatives with names should use them
temp_args = args + [
'--derivatives',
f'anat={str(bids_path / "derivatives/smriprep")}',
]
opts = parser.parse_args(temp_args)
assert opts.derivatives == {'anat': bids_path / 'derivatives/smriprep'}
_reset_config()

# Providing multiple unlabeled derivatives with the same name should raise an error
temp_args = args + [
'--derivatives',
str(bids_path / 'derivatives_01/smriprep'),
str(bids_path / 'derivatives_02/smriprep'),
]
with pytest.raises(ValueError, match='Received duplicate derivative name'):
parser.parse_args(temp_args)

_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

0 comments on commit ecf8580

Please sign in to comment.