diff --git a/nibabies/cli/parser.py b/nibabies/cli/parser.py index 14d3b678..572f7f60 100644 --- a/nibabies/cli/parser.py +++ b/nibabies/cli/parser.py @@ -28,6 +28,8 @@ def _build_parser(): # parser attribute name: (replacement flag, version slated to be removed in) 'bold2t1w_init': ('--bold2anat-init', '24.2.0'), 'bold2t1w_dof': ('--bold2anat-dof', '24.2.0'), + 'force_reconall': ('--surface-recon-method freesurfer', '24.2.0'), + 'fs_no_reconall': ('--surface-recon-method none', '24.2.0'), } class DeprecatedAction(Action): @@ -148,6 +150,11 @@ def _slice_time_ref(value, parser): raise parser.error(f'Slice time reference must be in range 0-1. Received {value}.') return value + def _str_none(val): + if not isinstance(val, str): + return val + return None if val.lower() == 'none' else val + verstr = f'NiBabies v{config.environment.version}' currentv = Version(config.environment.version) @@ -619,9 +626,9 @@ def _slice_time_ref(value, parser): ) g_surfs_xor.add_argument( '--fs-no-reconall', - action='store_false', + action=DeprecatedAction, dest='run_reconall', - help='disable FreeSurfer surface preprocessing.', + help='Deprecated - use `--surface-recon-method none` instead.', ) g_other = parser.add_argument_group('Other options') @@ -750,14 +757,15 @@ def _slice_time_ref(value, parser): g_baby.add_argument( '--force-reconall', default=False, - action='store_true', - help='Force traditional FreeSurfer surface reconstruction.', + action=DeprecatedAction, + help='Deprecated - use `--surface-recon-method freesurfer` instead.', ) g_baby.add_argument( '--surface-recon-method', - choices=('infantfs', 'freesurfer', 'mcribs', 'auto'), + choices=('auto', 'infantfs', 'freesurfer', 'mcribs', None), + type=_str_none, default='auto', - help='Method to use for surface reconstruction', + help='Method to use for surface reconstruction.', ) g_baby.add_argument( '--reference-anat', diff --git a/nibabies/cli/tests/test_parser.py b/nibabies/cli/tests/test_parser.py index f0caabf9..e2be6190 100644 --- a/nibabies/cli/tests/test_parser.py +++ b/nibabies/cli/tests/test_parser.py @@ -270,3 +270,34 @@ def test_derivatives(tmp_path): parser.parse_args(temp_args) _reset_config() + + +@pytest.mark.parametrize( + ('args', 'expectation'), + [ + ([], 'auto'), + (['--surface-recon-method', 'auto'], 'auto'), + (['--surface-recon-method', 'mcribs'], 'mcribs'), + (['--surface-recon-method', 'infantfs'], 'infantfs'), + (['--surface-recon-method', 'freesurfer'], 'freesurfer'), + (['--surface-recon-method', 'none'], None), + (['--surface-recon-method', 'None'], None), + (['--surface-recon-method', 123], (TypeError,)), + ], +) +def test_surface_recon_method(tmp_path, args, expectation): + """Check the correct parsing of the memory argument.""" + datapath = tmp_path / 'data' + datapath.mkdir(exist_ok=True) + _fs_file = tmp_path / 'license.txt' + _fs_file.write_text('') + + args = [str(datapath)] + MIN_ARGS[1:] + ['--fs-license-file', str(_fs_file)] + args + + cm = nullcontext() + if isinstance(expectation, tuple): + cm = pytest.raises(expectation) + + with cm: + opts = _build_parser().parse_args(args) + assert opts.surface_recon_method == expectation diff --git a/nibabies/workflows/base.py b/nibabies/workflows/base.py index 2742ee00..68226104 100644 --- a/nibabies/workflows/base.py +++ b/nibabies/workflows/base.py @@ -71,6 +71,9 @@ from niworkflows.utils.spaces import SpatialReferences +AUTO_T2W_MAX_AGE = 8 + + def init_nibabies_wf(subworkflows_list): """ Build *NiBabies*'s pipeline. @@ -105,8 +108,8 @@ def init_nibabies_wf(subworkflows_list): nibabies_wf.base_dir = config.execution.work_dir execution_spaces = init_execution_spaces() - freesurfer = config.workflow.surface_recon_method is not None - if freesurfer: + surface_recon = config.workflow.surface_recon_method is not None + if surface_recon: fsdir = pe.Node( BIDSFreeSurferDir( derivatives=config.execution.output_dir, @@ -154,7 +157,7 @@ def init_nibabies_wf(subworkflows_list): single_subject_wf.config['execution']['crashdump_dir'] = str(log_dir) for node in single_subject_wf._get_all_nodes(): node.config = deepcopy(single_subject_wf.config) - if freesurfer: + if surface_recon: nibabies_wf.connect(fsdir, 'subjects_dir', single_subject_wf, 'inputnode.subjects_dir') else: nibabies_wf.add_nodes([single_subject_wf]) @@ -327,30 +330,32 @@ def init_single_subject_wf( # Determine some session level options here, as we should have # all the required information if recon_method == 'auto': - if age <= 8: + if age <= AUTO_T2W_MAX_AGE and anatomical_cache.get('t2w_aseg'): + # do not force mcribs without a vetted segmentation recon_method = 'mcribs' elif age <= 24: recon_method = 'infantfs' else: recon_method = 'freesurfer' - preferred_anat = config.execution.reference_anat + requested_anat = config.execution.reference_anat t1w = subject_data['t1w'] t2w = subject_data['t2w'] single_anat = False - if not t1w and t2w: + + if not (t1w and t2w): single_anat = True reference_anat = 'T1w' if t1w else 'T2w' - if preferred_anat and reference_anat != preferred_anat: + if requested_anat and reference_anat != requested_anat: raise AttributeError( - f'Requested to use {preferred_anat} as anatomical reference but none available' + f'Requested to use {requested_anat} as anatomical reference but none available' ) - else: - if not (reference_anat := preferred_anat): - if recon_method is None: - reference_anat = 'T2w' if age <= 8 else 'T1w' - else: - reference_anat = 'T2w' if recon_method == 'mcribs' else 'T1w' + elif (reference_anat := requested_anat) is None: # Both available with no preference + reference_anat = ( + 'T2w' + if any((recon_method == 'none' and age <= AUTO_T2W_MAX_AGE, recon_method == 'mcribs')) + else 'T1w' + ) anat = reference_anat.lower() # To be used for workflow connections