diff --git a/.github/Dockerfiles/base-standard.Dockerfile b/.github/Dockerfiles/base-standard.Dockerfile index 1abba7948d..a03fd9a75a 100644 --- a/.github/Dockerfiles/base-standard.Dockerfile +++ b/.github/Dockerfiles/base-standard.Dockerfile @@ -27,7 +27,8 @@ RUN apt-get update \ && apt-get install --no-install-recommends -y bc \ && yes | mamba install tcsh \ && yes | mamba clean --all \ - && cp -l `which tcsh` /bin/tcsh + && cp -l `which tcsh` /bin/tcsh \ + && cp -l `which tcsh` /bin/csh ENV FREESURFER_HOME="/usr/lib/freesurfer" \ NO_FSFAST=1 ENV PATH="$FREESURFER_HOME/bin:$PATH" \ diff --git a/CHANGELOG.md b/CHANGELOG.md index e4b7b0492b..a18805ebd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Added - Some automatic handling of user-provided BIDSy atlas names. +- `sig_imports` static method decorator for `Function` nodes, to accommodate type hinting in signatures of `Function` node functions. ## Fixed diff --git a/CPAC/func_preproc/func_motion.py b/CPAC/func_preproc/func_motion.py index 18d8bd4559..ae8fcd7ccc 100644 --- a/CPAC/func_preproc/func_motion.py +++ b/CPAC/func_preproc/func_motion.py @@ -577,18 +577,7 @@ def motion_correct_mcflirt(wf, cfg, strat_pool, pipe_num): def motion_correct_connections(wf, cfg, strat_pool, pipe_num, opt): - """Check opt for valid option, then connect that option. - - Parameters - ---------- - passed through from node block - - Returns - ------- - wf : pe.Workflow - - outputs : dict - """ + """Check opt for valid option, then connect that option.""" if opt not in motion_correct_options: raise KeyError("\n\n[!] Error: The 'tool' parameter of the " "'motion_correction' workflow must be one of " @@ -626,8 +615,7 @@ def motion_correct_connections(wf, cfg, strat_pool, pipe_num, opt): "motion-filter-info": {}, "motion-filter-plot": {}}) def motion_estimate_filter(wf, cfg, strat_pool, pipe_num, opt=None): - ''' - Filter motion parameters. + '''Filter motion parameters. .. versionchanged:: 1.8.6 Beginning with version 1.8.6, C-PAC outputs both the unfiltered diff --git a/CPAC/func_preproc/func_preproc.py b/CPAC/func_preproc/func_preproc.py index bafa10c920..bb2aaef13c 100644 --- a/CPAC/func_preproc/func_preproc.py +++ b/CPAC/func_preproc/func_preproc.py @@ -171,8 +171,8 @@ def anat_refined_mask(init_bold_mask=True, wf_name='init_bold_mask'): def anat_based_mask(wf_name='bold_mask'): -# reference DCAN lab BOLD mask -# https://github.com/DCAN-Labs/DCAN-HCP/blob/master/fMRIVolume/scripts/DistortionCorrectionAndEPIToT1wReg_FLIRTBBRAndFreeSurferBBRbased.sh + """reference `DCAN lab BOLD mask `_ + """ wf = pe.Workflow(name=wf_name) input_node = pe.Node(util.IdentityInterface(fields=['func', @@ -252,22 +252,24 @@ def anat_based_mask(wf_name='bold_mask'): def create_scale_func_wf(scaling_factor, wf_name='scale_func'): """Workflow to scale func data. - Parameters - ---------- - scaling_factor : float - Scale the size of the dataset voxels by the factor. - wf_name : string - name of the workflow Workflow Inputs:: inputspec.func : func file or a list of func/rest nifti file User input functional(T2*) Image Workflow Outputs:: - outputspec.scaled_func : string (nifti file) + outputspec.scaled_func : str (nifti file) Path to Output image with scaled data + Order of commands: - Scale the size of the dataset voxels by the factor 'fac'. For details see `3dcalc `_:: 3drefit -xyzscale fac rest.nii.gz + + Parameters + ---------- + scaling_factor : float + Scale the size of the dataset voxels by the factor. + wf_name : str + name of the workflow """ # allocate a workflow object @@ -306,15 +308,15 @@ def create_wf_edit_func(wf_name="edit_func"): inputspec.func : func file or a list of func/rest nifti file User input functional(T2*) Image - inputspec.start_idx : string + inputspec.start_idx : str Starting volume/slice of the functional image (optional) - inputspec.stop_idx : string + inputspec.stop_idx : str Last volume/slice of the functional image (optional) Workflow Outputs:: - outputspec.edited_func : string (nifti file) + outputspec.edited_func : str (nifti file) Path to Output image with the initial few slices dropped @@ -463,7 +465,7 @@ def get_idx(in_files, stop_idx=None, start_idx=None): Parameters ---------- - in_file : string (nifti file) + in_file : str (nifti file) Path to input functional run stop_idx : int @@ -913,8 +915,9 @@ def form_thr_string(thr): outputs=['space-bold_desc-brain_mask', 'desc-ref_bold'] ) def bold_mask_fsl_afni(wf, cfg, strat_pool, pipe_num, opt=None): - # fMRIPrep-style BOLD mask - # Ref: https://github.com/nipreps/niworkflows/blob/maint/1.3.x/niworkflows/func/util.py#L246-L514 + """fMRIPrep-style BOLD mask + `Ref `_ + """ # Initialize transforms with antsAI init_aff = pe.Node( @@ -1210,8 +1213,7 @@ def bold_mask_anatomical_refined(wf, cfg, strat_pool, pipe_num, opt=None): ) def bold_mask_anatomical_based(wf, cfg, strat_pool, pipe_num, opt=None): '''Generate the BOLD mask by basing it off of the anatomical brain mask. - Adapted from DCAN Lab's BOLD mask method from the ABCD pipeline. - https://github.com/DCAN-Labs/DCAN-HCP/blob/master/fMRIVolume/scripts/DistortionCorrectionAndEPIToT1wReg_FLIRTBBRAndFreeSurferBBRbased.sh + Adapted from `DCAN Lab's BOLD mask method from the ABCD pipeline `_. ''' # 0. Take single volume of func @@ -1303,8 +1305,7 @@ def bold_mask_anatomical_based(wf, cfg, strat_pool, pipe_num, opt=None): ) def bold_mask_anatomical_resampled(wf, cfg, strat_pool, pipe_num, opt=None): '''Resample anatomical brain mask in standard space to get BOLD brain mask in standard space - Adapted from DCAN Lab's BOLD mask method from the ABCD pipeline. - https://github.com/DCAN-Labs/DCAN-HCP/blob/master/fMRIVolume/scripts/OneStepResampling.sh#L121-L132 + Adapted from `DCAN Lab's BOLD mask method from the ABCD pipeline `_. ''' # applywarp --rel --interp=spline -i ${T1wImage} -r ${ResampRefIm} --premat=$FSLDIR/etc/flirtsch/ident.mat -o ${WD}/${T1wImageFile}.${FinalfMRIResolution} @@ -1372,8 +1373,7 @@ def bold_mask_anatomical_resampled(wf, cfg, strat_pool, pipe_num, opt=None): ) def bold_mask_ccs(wf, cfg, strat_pool, pipe_num, opt=None): '''Generate the BOLD mask by basing it off of the anatomical brain. - Adapted from the BOLD mask method from the CCS pipeline. - https://github.com/TingsterX/CCS/blob/master/ccs_01_funcpreproc.sh#L89-L110 + Adapted from `the BOLD mask method from the CCS pipeline `_. ''' # Run 3dAutomask to generate func initial mask diff --git a/CPAC/generate_motion_statistics/generate_motion_statistics.py b/CPAC/generate_motion_statistics/generate_motion_statistics.py index 3a9ea9eef7..69ab477d14 100644 --- a/CPAC/generate_motion_statistics/generate_motion_statistics.py +++ b/CPAC/generate_motion_statistics/generate_motion_statistics.py @@ -17,10 +17,6 @@ """Functions to calculate motion statistics""" import os import sys -try: - from typing import Literal -except ImportError: - from typing_extensions import Literal from typing import Optional import numpy as np import nibabel as nb @@ -30,6 +26,7 @@ from CPAC.pipeline import nipype_pipeline_engine as pe from CPAC.utils.interfaces.function import Function from CPAC.utils.pytest import skipif +from CPAC.utils.typing import LITERAL def motion_power_statistics(name='motion_stats', @@ -351,9 +348,14 @@ def calculate_FD_P(in_file): return out_file +@Function.sig_imports(['import os', 'import sys', + 'from typing import Optional', + 'import numpy as np', + 'from CPAC.utils.pytest import skipif', + 'from CPAC.utils.typing import LITERAL']) @skipif(sys.version_info < (3, 10), reason="Test requires Python 3.10 or higher") -def calculate_FD_J(in_file: str, calc_from: Literal['affine', 'rms'], +def calculate_FD_J(in_file: str, calc_from: LITERAL['affine', 'rms'], center: Optional[np.ndarray] = None) -> str: """ Method to calculate framewise displacement as per Jenkinson et al. 2002 diff --git a/CPAC/pipeline/nodeblock.py b/CPAC/pipeline/nodeblock.py index 0669334590..62c3506f6f 100644 --- a/CPAC/pipeline/nodeblock.py +++ b/CPAC/pipeline/nodeblock.py @@ -54,10 +54,32 @@ def __init__( self.__name__ = func.__name__ self.__qualname__ = func.__qualname__ self.__annotations__ = func.__annotations__ - self.__doc__ = func.__doc__ + self.__doc__ = ''.join([_.replace(' ', '') for _ in [ + func.__doc__, '', '', NodeBlockFunction.__call__.__doc__ + ] if _ is not None]).rstrip() # all node block functions have this signature def __call__(self, wf, cfg, strat_pool, pipe_num, opt=None): + """ + + Parameters + ---------- + wf : ~nipype.pipeline.engine.workflows.Workflow + + cfg : ~CPAC.utils.configuration.Configuration + + strat_pool + + pipe_num : int + + opt : str, optional + + Returns + ------- + wf : ~nipype.pipeline.engine.workflows.Workflow + + out : dict + """ return self.func(wf, cfg, strat_pool, pipe_num, opt) def legacy_nodeblock_dict(self): diff --git a/CPAC/pipeline/utils.py b/CPAC/pipeline/utils.py index 00979da64f..5fa1560979 100644 --- a/CPAC/pipeline/utils.py +++ b/CPAC/pipeline/utils.py @@ -85,10 +85,10 @@ def present_outputs(outputs: dict, keys: list) -> dict: """ Given an outputs dictionary and a list of output keys, returns the subset of ``outputs`` that includes all keys in ``keys`` that are - present. I.e., ~:py:func:`CPAC.func_preproc.func_motion.motion_correct_connections` + present. I.e., :py:func:`~CPAC.func_preproc.func_motion.motion_correct_connections` will have different items in its ``outputs`` dictionary at different times depending on the ``motion_correction`` configuration; - ~:py:func:`CPAC.func_preproc.func_motion.func_motion_estimates` can + :py:func:`~CPAC.func_preproc.func_motion.func_motion_estimates` can then wrap that ``outputs`` in this function and provide a list of keys of the desired outputs to include, if they are present in the provided ``outputs`` dictionary, eliminating the need for multiple diff --git a/CPAC/qc/qcmetrics.py b/CPAC/qc/qcmetrics.py index 6db977c495..f1c68d2f67 100644 --- a/CPAC/qc/qcmetrics.py +++ b/CPAC/qc/qcmetrics.py @@ -55,8 +55,8 @@ def dc(input1, input2): """ input1 = nb.load(input1).get_fdata() input2 = nb.load(input2).get_fdata() - input1 = np.atleast_1d(input1.astype(np.bool)) - input2 = np.atleast_1d(input2.astype(np.bool)) + input1 = np.atleast_1d(input1.astype(bool)) + input2 = np.atleast_1d(input2.astype(bool)) intersection = np.count_nonzero(input1 & input2) @@ -95,8 +95,8 @@ def jc(input1, input2): """ input1 = nb.load(input1).get_fdata() input2 = nb.load(input2).get_fdata() - input1 = np.atleast_1d(input1.astype(np.bool)) - input2 = np.atleast_1d(input2.astype(np.bool)) + input1 = np.atleast_1d(input1.astype(bool)) + input2 = np.atleast_1d(input2.astype(bool)) intersection = np.count_nonzero(input1 & input2) union = np.count_nonzero(input1 | input2) @@ -110,8 +110,8 @@ def crosscorr(input1, input2): r"""cross correlation: compute cross correction bewteen input masks""" input1 = nb.load(input1).get_fdata() input2 = nb.load(input2).get_fdata() - input1 = np.atleast_1d(input1.astype(np.bool)).flatten() - input2 = np.atleast_1d(input2.astype(np.bool)).flatten() + input1 = np.atleast_1d(input1.astype(bool)).flatten() + input2 = np.atleast_1d(input2.astype(bool)).flatten() cc = np.corrcoef(input1, input2)[0][1] return cc @@ -120,8 +120,8 @@ def coverage(input1, input2): """Estimate the coverage between two masks.""" input1 = nb.load(input1).get_fdata() input2 = nb.load(input2).get_fdata() - input1 = np.atleast_1d(input1.astype(np.bool)) - input2 = np.atleast_1d(input2.astype(np.bool)) + input1 = np.atleast_1d(input1.astype(bool)) + input2 = np.atleast_1d(input2.astype(bool)) intsec = np.count_nonzero(input1 & input2) if np.sum(input1) > np.sum(input2): smallv = np.sum(input2) diff --git a/CPAC/qc/utils.py b/CPAC/qc/utils.py index 8f9a59f727..6b3f8f2d78 100644 --- a/CPAC/qc/utils.py +++ b/CPAC/qc/utils.py @@ -1262,8 +1262,8 @@ def dc(input1, input2): ----- This is a real metric. """ - input1 = numpy.atleast_1d(input1.astype(numpy.bool)) - input2 = numpy.atleast_1d(input2.astype(numpy.bool)) + input1 = numpy.atleast_1d(input1.astype(bool)) + input2 = numpy.atleast_1d(input2.astype(bool)) intersection = numpy.count_nonzero(input1 & input2) @@ -1303,8 +1303,8 @@ def jc(input1, input2): ----- This is a real metric. """ - input1 = numpy.atleast_1d(input1.astype(numpy.bool)) - input2 = numpy.atleast_1d(input2.astype(numpy.bool)) + input1 = numpy.atleast_1d(input1.astype(bool)) + input2 = numpy.atleast_1d(input2.astype(bool)) intersection = numpy.count_nonzero(input1 & input2) union = numpy.count_nonzero(input1 | input2) @@ -1320,8 +1320,8 @@ def crosscorr(input1,input2): computer compute cross correction bewteen input mask """ - input1 = numpy.atleast_1d(input1.astype(numpy.bool)) - input2 = numpy.atleast_1d(input2.astype(numpy.bool)) + input1 = numpy.atleast_1d(input1.astype(bool)) + input2 = numpy.atleast_1d(input2.astype(bool)) from scipy.stats.stats import pearsonr cc=pearsonr(input1,input2) @@ -1331,8 +1331,8 @@ def coverage(input1,input2): """ estimate the coverage between two mask """ - input1 = numpy.atleast_1d(input1.astype(numpy.bool)) - input2 = numpy.atleast_1d(input2.astype(numpy.bool)) + input1 = numpy.atleast_1d(input1.astype(bool)) + input2 = numpy.atleast_1d(input2.astype(bool)) intsec=numpy.count_nonzero(input1 & input2) if numpy.sum(input1)> numpy.sum(input2): diff --git a/CPAC/surface/tests/test_installation.py b/CPAC/surface/tests/test_installation.py index f36b3f7009..869d6f3d6d 100644 --- a/CPAC/surface/tests/test_installation.py +++ b/CPAC/surface/tests/test_installation.py @@ -20,10 +20,12 @@ from CPAC.utils.monitoring.custom_logging import log_subprocess +@pytest.mark.parametrize("executable", ["bc", "csh"]) @pytest.mark.skipif("FREESURFER_HOME" not in os.environ or not os.path.exists(os.environ['FREESURFER_HOME']), - reason="We don't need bc if we don't have FreeSurfer.") -def test_bc(): - """Make sure ``bc`` is installed""" - _, exit_code = log_subprocess(['bc', '--version']) + reason="We don't need these dependencies if we don't" + "have FreeSurfer.") +def test_executable(executable): + """Make sure executable is installed""" + _, exit_code = log_subprocess([executable, "--version"]) assert exit_code == 0 diff --git a/CPAC/utils/interfaces/function/function.py b/CPAC/utils/interfaces/function/function.py index 9117598ae5..977b802606 100644 --- a/CPAC/utils/interfaces/function/function.py +++ b/CPAC/utils/interfaces/function/function.py @@ -1,5 +1,6 @@ from builtins import str, bytes import inspect +from typing import Callable, List from nipype import logging from nipype.interfaces.base import (traits, DynamicTraitedSpec, Undefined, @@ -57,11 +58,22 @@ def __init__(self, parameter) imports : list of strings list of import statements that allow the function to execute - in an otherwise empty namespace + in an otherwise empty namespace. If these collide with + imports defined via the :py:meth:`Function.sig_imports` + decorator, the imports given as a parameter here will take + precedence over those from the decorator. """ - super(Function, self).__init__(**inputs) + super().__init__(**inputs) if function: + if hasattr(function, 'ns_imports'): + # prepend the ns_imports from the decorator to + # the paramater imports + _ns_imports = [ + 'from CPAC.utils.interfaces.function import Function', + *function.ns_imports] + imports = _ns_imports if imports is None else [*_ns_imports, + *imports] if as_module: module = inspect.getmodule(function).__name__ full_name = "%s.%s" % (module, function.__name__) @@ -96,6 +108,58 @@ def __init__(self, for name in self._output_names: self._out[name] = None + @staticmethod + def sig_imports(imports: List[str]) -> Callable: + """ + Sets an ``ns_imports`` attribute on a function for + Function-node functions. + This can be useful for classes needed for decorators, typehints + and for avoiding redefinitions. + + Parameters + ---------- + imports : list of str + import statements to import the function in an otherwise empty + namespace. If these collide with imports defined via the + :py:meth:`Function.__init__` initialization method, the + imports given as a parameter here will be overridden by + those from the initializer. + + Returns + ------- + func : function + + Examples + -------- + See the defintion of + :py:func:`~CPAC.generate_motion_statistics.calculate_FD_J` to see the + decorator tested here being applied. + >>> from CPAC.generate_motion_statistics import calculate_FD_J + >>> calc_fdj = Function(input_names=['in_file', 'calc_from', 'center'], + ... output_names=['out_file'], + ... function=calculate_FD_J, + ... as_module=True) + >>> calc_fdj.imports # doctest: +NORMALIZE_WHITESPACE + ['from CPAC.utils.interfaces.function import Function', + 'import os', + 'import sys', + 'from typing import Optional', + 'import numpy as np', + 'from CPAC.utils.pytest import skipif', + 'from CPAC.utils.typing import LITERAL'] + >>> from inspect import signature + >>> from nipype.utils.functions import (getsource, + ... create_function_from_source) + >>> f = create_function_from_source(getsource(calculate_FD_J), + ... calc_fdj.imports) + >>> inspect.signature(calculate_FD_J) == inspect.signature(f) + True + """ + def _imports(func: Callable) -> Callable: + setattr(func, 'ns_imports', imports) + return func + return _imports + def _set_function_string(self, obj, name, old, new): if name == 'function_str': if self.as_module: diff --git a/CPAC/utils/typing.py b/CPAC/utils/typing.py index d623e01217..01da1fc1b6 100644 --- a/CPAC/utils/typing.py +++ b/CPAC/utils/typing.py @@ -25,6 +25,12 @@ from CPAC.utils.docs import DOCS_URL_PREFIX __doc__ = __doc__.replace(r'{DOCS_URL_PREFIX}', DOCS_URL_PREFIX) +if sys.version_info >= (3, 8): + from typing import Literal + LITERAL = Literal +else: + from typing_extensions import Literal + LITERAL = Literal if sys.version_info >= (3, 10): LIST_OR_STR = list | str TUPLE = tuple @@ -33,4 +39,4 @@ LIST_OR_STR = Union[list, str] TUPLE = Tuple -__all__ = ['LIST_OR_STR', 'TUPLE'] +__all__ = ['LIST_OR_STR', 'LITERAL', 'TUPLE']