diff --git a/xcp_d/cli/parser.py b/xcp_d/cli/parser.py index 3a8294870..0a28734c3 100644 --- a/xcp_d/cli/parser.py +++ b/xcp_d/cli/parser.py @@ -70,7 +70,7 @@ def _build_parser(): '--mode', dest='mode', action='store', - choices=['abcd', 'hbcd', 'linc', 'none'], + choices=['abcd', 'hbcd', 'linc', 'nichart', 'none'], required=True, help=( 'The mode of operation for XCP-D. ' @@ -285,9 +285,10 @@ def _build_parser(): g_param.add_argument( '--smoothing', dest='smoothing', - default=6, + default='auto', action='store', - type=float, + type=parser_utils._float_or_auto, + metavar='{{auto,FLOAT}}', help=( 'FWHM, in millimeters, of the Gaussian smoothing kernel to apply to the denoised BOLD ' 'data. ' @@ -531,13 +532,13 @@ def _build_parser(): '--min_coverage', dest='min_coverage', required=False, - default=0.5, + default='auto', type=parser_utils._restricted_float, + metavar='{auto,FLOAT}', help=( 'Coverage threshold to apply to parcels in each atlas. ' 'Any parcels with lower coverage than the threshold will be replaced with NaNs. ' - 'Must be a value between zero and one, indicating proportion of the parcel. ' - 'Default is 0.5.' + 'Must be a value between zero and one, indicating proportion of the parcel.' ), ) @@ -939,7 +940,13 @@ def _validate_parameters(opts, build_log, parser): assert opts.despike in (True, False, 'auto') assert opts.file_format in ('nifti', 'cifti', 'auto') assert opts.linc_qc in (True, False, 'auto') - assert opts.mode in ('abcd', 'hbcd', 'linc', 'none'), f"Unsupported mode '{opts.mode}'." + assert opts.mode in ( + 'abcd', + 'hbcd', + 'linc', + 'nichart', + 'none', + ), f'Unsupported mode "{opts.mode}".' assert opts.output_type in ('censored', 'interpolated', 'auto') assert opts.process_surfaces in (True, False, 'auto') @@ -968,12 +975,14 @@ def _validate_parameters(opts, build_log, parser): opts.file_format = 'cifti' if (opts.file_format == 'auto') else opts.file_format opts.input_type = 'fmriprep' if opts.input_type == 'auto' else opts.input_type opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc + opts.min_coverage = 0.5 if opts.min_coverage == 'auto' else opts.min_coverage if opts.motion_filter_type is None: error_messages.append(f"'--motion-filter-type' is required for '{opts.mode}' mode.") opts.output_correlations = True if 'all' in opts.dcan_correlation_lengths else False if opts.output_type == 'censored': error_messages.append(f"'--output-type' cannot be 'censored' for '{opts.mode}' mode.") opts.output_type = 'interpolated' + opts.smoothing = 6 if opts.smoothing == 'auto' else opts.smoothing opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config opts.process_surfaces = ( True if (opts.process_surfaces == 'auto') else opts.process_surfaces @@ -994,12 +1003,14 @@ def _validate_parameters(opts, build_log, parser): opts.file_format = 'cifti' if (opts.file_format == 'auto') else opts.file_format opts.input_type = 'nibabies' if opts.input_type == 'auto' else opts.input_type opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc + opts.min_coverage = 0.5 if opts.min_coverage == 'auto' else opts.min_coverage if opts.motion_filter_type is None: error_messages.append(f"'--motion-filter-type' is required for '{opts.mode}' mode.") opts.output_correlations = True if 'all' in opts.dcan_correlation_lengths else False if opts.output_type == 'censored': error_messages.append(f"'--output-type' cannot be 'censored' for '{opts.mode}' mode.") opts.output_type = 'interpolated' + opts.smoothing = 6 if opts.smoothing == 'auto' else opts.smoothing opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config opts.process_surfaces = ( True if (opts.process_surfaces == 'auto') else opts.process_surfaces @@ -1017,16 +1028,40 @@ def _validate_parameters(opts, build_log, parser): opts.file_format = 'cifti' if (opts.file_format == 'auto') else opts.file_format opts.input_type = 'fmriprep' if opts.input_type == 'auto' else opts.input_type opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc + opts.min_coverage = 0.5 if opts.min_coverage == 'auto' else opts.min_coverage opts.output_correlations = True if opts.output_type == 'interpolated': error_messages.append( f"'--output-type' cannot be 'interpolated' for '{opts.mode}' mode." ) opts.output_type = 'censored' + opts.smoothing = 6 if opts.smoothing == 'auto' else opts.smoothing opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config opts.process_surfaces = False if opts.process_surfaces == 'auto' else opts.process_surfaces if opts.dcan_correlation_lengths is not None: error_messages.append(f"'--create-matrices' is not supported for '{opts.mode}' mode.") + elif opts.mode == 'nichart': + opts.abcc_qc = False if (opts.abcc_qc == 'auto') else opts.abcc_qc + opts.combine_runs = False if opts.combine_runs == 'auto' else opts.combine_runs + opts.confounds_config = ( + '36P' if (opts.confounds_config == 'auto') else opts.confounds_config + ) + opts.dcan_correlation_lengths = ( + 'all' if opts.dcan_correlation_lengths is None else opts.dcan_correlation_lengths + ) + opts.despike = True if (opts.despike == 'auto') else opts.despike + opts.fd_thresh = 0 if (opts.fd_thresh == 'auto') else opts.fd_thresh + opts.file_format = 'nifti' if (opts.file_format == 'auto') else opts.file_format + opts.input_type = 'fmriprep' if opts.input_type == 'auto' else opts.input_type + opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc + opts.min_coverage = 0.4 if opts.min_coverage == 'auto' else opts.min_coverage + opts.output_correlations = True if 'all' in opts.dcan_correlation_lengths else False + opts.output_type = 'censored' if opts.output_type == 'auto' else opts.output_type + opts.smoothing = 0 if opts.smoothing == 'auto' else opts.smoothing + opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config + opts.process_surfaces = False if opts.process_surfaces == 'auto' else opts.process_surfaces + # Remove "all" from the list of correlation lengths + opts.dcan_correlation_lengths = [c for c in opts.dcan_correlation_lengths if c != 'all'] elif opts.mode == 'none': if opts.abcc_qc == 'auto': error_messages.append("'--abcc-qc' (y or n) is required for 'none' mode.") @@ -1057,6 +1092,9 @@ def _validate_parameters(opts, build_log, parser): if opts.linc_qc == 'auto': error_messages.append("'--linc-qc' (y or n) is required for 'none' mode.") + if opts.min_coverage == 'auto': + error_messages.append("'--min-coverage' is required for 'none' mode.") + if opts.motion_filter_type is None: error_messages.append("'--motion-filter-type' is required for 'none' mode.") @@ -1070,6 +1108,9 @@ def _validate_parameters(opts, build_log, parser): "'--warp-surfaces-native2std' (y or n) is required for 'none' mode." ) + if opts.smoothing == 'auto': + error_messages.append("'--smoothing' is required for 'none' mode.") + # Remove "all" from the list of correlation lengths opts.dcan_correlation_lengths = [c for c in opts.dcan_correlation_lengths if c != 'all'] diff --git a/xcp_d/cli/parser_utils.py b/xcp_d/cli/parser_utils.py index a11dd2f55..84ed32201 100644 --- a/xcp_d/cli/parser_utils.py +++ b/xcp_d/cli/parser_utils.py @@ -66,6 +66,9 @@ def _float_or_auto_or_none(string, is_parser=True): def _restricted_float(x): """From https://stackoverflow.com/a/12117065/2589328.""" + if x == 'auto': + return x + try: x = float(x) except ValueError as exc: diff --git a/xcp_d/tests/test_cli.py b/xcp_d/tests/test_cli.py index 9f96f7619..090038c0c 100644 --- a/xcp_d/tests/test_cli.py +++ b/xcp_d/tests/test_cli.py @@ -60,7 +60,9 @@ def test_ds001419_nifti(data_dir, output_dir, working_dir): '--motion-filter-type=lp', '--band-stop-min=6', '--skip-parcellation', + '--min-coverage=0.4', '--min-time=100', + '--smoothing=6', '--combine-runs', '--output-type=censored', '--combine-runs=y', diff --git a/xcp_d/tests/test_cli_run.py b/xcp_d/tests/test_cli_run.py index 9733b256e..3faeed277 100644 --- a/xcp_d/tests/test_cli_run.py +++ b/xcp_d/tests/test_cli_run.py @@ -45,10 +45,12 @@ def base_opts(): 'motion_filter_order': None, 'process_surfaces': 'auto', 'atlases': ['Glasser'], + 'min_coverage': 'auto', 'dcan_correlation_lengths': None, 'despike': 'auto', 'abcc_qc': 'auto', 'linc_qc': 'auto', + 'smoothing': 'auto', 'combine_runs': 'auto', 'output_type': 'auto', 'fs_license_file': None, @@ -276,6 +278,8 @@ def test_validate_parameters_linc_mode(base_opts, base_parser, capsys): assert opts.abcc_qc is False assert opts.linc_qc is True assert opts.file_format == 'cifti' + assert opts.min_coverage == 0.5 + assert opts.smoothing == 6.0 # --create-matrices is not supported opts.dcan_correlation_lengths = [300] @@ -304,8 +308,10 @@ def test_validate_parameters_abcd_mode(base_opts, base_parser, capsys): assert opts.file_format == 'cifti' assert opts.input_type == 'fmriprep' assert opts.linc_qc is True + assert opts.min_coverage == 0.5 assert opts.output_correlations is False assert opts.process_surfaces is True + assert opts.smoothing == 6.0 opts.dcan_correlation_lengths = ['300', 'all'] opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser) @@ -339,8 +345,10 @@ def test_validate_parameters_hbcd_mode(base_opts, base_parser, capsys): assert opts.file_format == 'cifti' assert opts.input_type == 'nibabies' assert opts.linc_qc is True + assert opts.min_coverage == 0.5 assert opts.output_correlations is False assert opts.process_surfaces is True + assert opts.smoothing == 6.0 opts.dcan_correlation_lengths = ['300', 'all'] opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser) @@ -356,6 +364,21 @@ def test_validate_parameters_hbcd_mode(base_opts, base_parser, capsys): assert "'--motion-filter-type' is required for" in stderr +def test_validate_parameters_nichart_mode(base_opts, base_parser, capsys): + """Test parser._validate_parameters with nichart mode.""" + opts = deepcopy(base_opts) + opts.mode = 'nichart' + + # linc mode doesn't use abcc_qc but does use linc_qc + opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser) + + assert opts.abcc_qc is False + assert opts.linc_qc is True + assert opts.file_format == 'nifti' + assert opts.min_coverage == 0.4 + assert opts.smoothing == 0 + + def test_validate_parameters_none_mode(base_opts, base_parser, capsys): """Test parser._validate_parameters with none mode.""" opts = deepcopy(base_opts) @@ -372,9 +395,11 @@ def test_validate_parameters_none_mode(base_opts, base_parser, capsys): assert "'--file-format' is required for 'none' mode." in stderr assert "'--input-type' is required for 'none' mode." in stderr assert "'--linc-qc' (y or n) is required for 'none' mode." in stderr + assert "'--min-coverage' is required for 'none' mode." in stderr assert "'--motion-filter-type' is required for 'none' mode." in stderr assert "'--nuisance-regressors' is required for 'none' mode." in stderr assert "'--output-type' is required for 'none' mode." in stderr + assert "'--smoothing' is required for 'none' mode." in stderr assert "'--warp-surfaces-native2std' (y or n) is required for 'none' mode." in stderr opts.abcc_qc = False @@ -385,10 +410,12 @@ def test_validate_parameters_none_mode(base_opts, base_parser, capsys): opts.file_format = 'nifti' opts.input_type = 'fmriprep' opts.linc_qc = False + opts.min_coverage = 0.5 opts.motion_filter_type = 'none' opts.output_type = 'censored' opts.params = '36P' opts.process_surfaces = False + opts.smoothing = 0 opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser) @@ -397,7 +424,7 @@ def test_validate_parameters_other_mode(base_opts, base_parser, capsys): opts = deepcopy(base_opts) opts.mode = 'other' - with pytest.raises(AssertionError, match="Unsupported mode 'other'"): + with pytest.raises(AssertionError, match='Unsupported mode "other"'): parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser)