Merge pull request #2857 from mgxd/sty/pre-commit

MAINT: Add `pre-commit`, dev installation for consistent styling

Author: mgxd

.git-blame-ignore-revs

@@ -1,3 +1,7 @@
+# 2022-09-22 - [email protected] - STY: black/isort the docker wrapper
+9976458388f369cba4b7d81359acc40b52f6621c
+# 2022-09-22 - [email protected] - STY: Apply black/isort to codebase
+7eedab8b29497cee1d8dd540c2266e740f484a86
 # 2021-11-05 - [email protected] - STY: Update black config
 d2ad20301306f283d504ec7b5a1bd73ce58c2b11
 # 2021-09-22 - [email protected] - sty: run black

.github/workflows/contrib.yml

@@ -23,19 +23,20 @@ jobs:
         python-version: [3.9]
 
     steps:
-      - uses: actions/checkout@v2
-        with:
-          submodules: recursive
-          fetch-depth: 0
+      - uses: actions/checkout@v3
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v4
         with:
           python-version: ${{ matrix.python-version }}
       - name: Display Python version
         run: python -c "import sys; print(sys.version)"
-      - name: Install flake8
-        run: python -m pip install flake8
+      - name: Install black/isort
+        run: python -m pip install black isort
       - name: Check fMRIPrep
-        run: python -m flake8 fmriprep
+        run: |
+          python -m black --check fmriprep
+          python -m isort --check fmriprep
       - name: Check wrapper
-        run: python -m flake8 wrapper
+        run: |
+          python -m black --check wrapper
+          python -m isort --check wrapper

.pre-commit-config.yaml

@@ -0,0 +1,18 @@
+repos:
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v2.4.0
+    hooks:
+    -   id: trailing-whitespace
+    -   id: end-of-file-fixer
+    -   id: check-yaml
+    -   id: check-added-large-files
+-   repo: https://github.com/psf/black
+    rev: 22.3.0
+    hooks:
+    -   id: black
+        files: ^fmriprep/
+-   repo: https://github.com/pycqa/isort
+    rev: 5.10.1
+    hooks:
+    -   id: isort
+        files: ^fmriprep/

fmriprep/__about__.py

@@ -22,15 +22,18 @@
 #
 """Base module variables."""
 from ._version import get_versions
+
 __version__ = get_versions()['version']
 del get_versions
 
 __packagename__ = 'fmriprep'
 __copyright__ = 'Copyright 2022, The NiPreps Developers'
-__credits__ = ('Contributors: please check the ``.zenodo.json`` file at the top-level folder'
-               'of the repository')
+__credits__ = (
+    'Contributors: please check the ``.zenodo.json`` file at the top-level folder'
+    'of the repository'
+)
 __url__ = 'https://github.com/nipreps/fmriprep'
 
-DOWNLOAD_URL = (
-    'https://github.com/nipreps/{name}/archive/{ver}.tar.gz'.format(
-        name=__packagename__, ver=__version__))
+DOWNLOAD_URL = 'https://github.com/nipreps/{name}/archive/{ver}.tar.gz'.format(
+    name=__packagename__, ver=__version__
+)

fmriprep/__init__.py

@@ -2,12 +2,7 @@
 # vi: set ft=python sts=4 ts=4 sw=4 et:
 """Top-module metadata."""
 
-from .__about__ import (
-    __copyright__,
-    __credits__,
-    __packagename__,
-    __version__,
-)
+from .__about__ import __copyright__, __credits__, __packagename__, __version__
 
 __all__ = [
     '__copyright__',
@@ -19,8 +14,9 @@
 # Silence PyBIDS warning for extension entity behavior
 # Can be removed once minimum PyBIDS dependency hits 0.14
 try:
-    from packaging.version import Version
     import bids
+    from packaging.version import Version
+
     if Version(bids.__version__) < Version('0.14'):
         bids.config.set_option('extension_initial_dot', True)
 except (ImportError, ValueError):

fmriprep/__main__.py

@@ -24,7 +24,9 @@
 
 if __name__ == '__main__':
     import sys
+
     from . import __name__ as module
+
     # `python -m <module>` typically displays the command as __main__.py
     if '__main__.py' in sys.argv[0]:
         sys.argv[0] = '%s -m %s' % (sys.executable, module)

fmriprep/_warnings.py

@@ -21,8 +21,8 @@
 #     https://www.nipreps.org/community/licensing/
 #
 """Manipulate Python warnings."""
-import warnings
 import logging
+import warnings
 
 _wlog = logging.getLogger("py.warnings")
 _wlog.addHandler(logging.NullHandler())
@@ -34,9 +34,7 @@ def _warn(message, category=None, stacklevel=1, source=None):
         category = type(category).__name__
         category = category.replace("type", "WARNING")
 
-    logging.getLogger("py.warnings").warning(
-        f"{category or 'WARNING'}: {message}"
-    )
+    logging.getLogger("py.warnings").warning(f"{category or 'WARNING'}: {message}")
 
 
 def _showwarning(message, category, filename, lineno, file=None, line=None):

fmriprep/cli/parser.py

@@ -22,6 +22,7 @@
 #
 """Parser."""
 import sys
+
 from .. import config
 
 
@@ -30,15 +31,14 @@ def _build_parser(**kwargs):
 
     ``kwargs`` are passed to ``argparse.ArgumentParser`` (mainly useful for debugging).
     """
+    from argparse import ArgumentDefaultsHelpFormatter, ArgumentParser
     from functools import partial
     from pathlib import Path
-    from argparse import (
-        ArgumentParser,
-        ArgumentDefaultsHelpFormatter,
-    )
+
+    from niworkflows.utils.spaces import OutputReferencesAction, Reference
     from packaging.version import Version
+
     from .version import check_latest, is_flagged
-    from niworkflows.utils.spaces import Reference, OutputReferencesAction
 
     def _path_exists(path, parser):
         """Ensure a given path exists."""
@@ -61,25 +61,24 @@ def _min_one(value, parser):
         return value
 
     def _to_gb(value):
-        scale = {"G": 1, "T": 10 ** 3, "M": 1e-3, "K": 1e-6, "B": 1e-9}
+        scale = {"G": 1, "T": 10**3, "M": 1e-3, "K": 1e-6, "B": 1e-9}
         digits = "".join([c for c in value if c.isdigit()])
-        units = value[len(digits):] or "M"
+        units = value[len(digits) :] or "M"
         return int(digits) * scale[units[0]]
 
     def _drop_sub(value):
         return value[4:] if value.startswith("sub-") else value
 
     def _filter_pybids_none_any(dct):
         import bids
+
         return {
-            k: bids.layout.Query.NONE
-            if v is None
-            else (bids.layout.Query.ANY if v == "*" else v)
+            k: bids.layout.Query.NONE if v is None else (bids.layout.Query.ANY if v == "*" else v)
             for k, v in dct.items()
         }
 
     def _bids_filter(value, parser):
-        from json import loads, JSONDecodeError
+        from json import JSONDecodeError, loads
 
         if value:
             if Path(value).exists():
@@ -98,17 +97,16 @@ def _slice_time_ref(value, parser):
         try:
             value = float(value)
         except ValueError:
-            raise parser.error("Slice time reference must be number, 'start', or 'middle'. "
-                               f"Received {value}.")
+            raise parser.error(
+                "Slice time reference must be number, 'start', or 'middle'. " f"Received {value}."
+            )
         if not 0 <= value <= 1:
             raise parser.error(f"Slice time reference must be in range 0-1. Received {value}.")
         return value
 
     verstr = f"fMRIPrep v{config.environment.version}"
     currentv = Version(config.environment.version)
-    is_release = not any(
-        (currentv.is_devrelease, currentv.is_prerelease, currentv.is_postrelease)
-    )
+    is_release = not any((currentv.is_devrelease, currentv.is_prerelease, currentv.is_postrelease))
 
     parser = ArgumentParser(
         description="fMRIPrep: fMRI PREProcessing workflows v{}".format(
@@ -206,7 +204,7 @@ def _slice_time_ref(value, parser):
         metavar="PATH",
         type=Path,
         help="Path to a PyBIDS database folder, for faster indexing (especially "
-             "useful for large datasets). Will be created if not present."
+        "useful for large datasets). Will be created if not present.",
     )
 
     g_perfm = parser.add_argument_group("Options to handle performance")
@@ -239,8 +237,7 @@ def _slice_time_ref(value, parser):
     g_perfm.add_argument(
         "--low-mem",
         action="store_true",
-        help="attempt to reduce memory usage (will increase disk usage "
-        "in working directory)",
+        help="attempt to reduce memory usage (will increase disk usage " "in working directory)",
     )
     g_perfm.add_argument(
         "--use-plugin",
@@ -250,9 +247,7 @@ def _slice_time_ref(value, parser):
         type=IsFile,
         help="nipype plugin configuration file",
     )
-    g_perfm.add_argument(
-        "--anat-only", action="store_true", help="run anatomical workflows only"
-    )
+    g_perfm.add_argument("--anat-only", action="store_true", help="run anatomical workflows only")
     g_perfm.add_argument(
         "--boilerplate_only",
         action="store_true",
@@ -321,7 +316,7 @@ def _slice_time_ref(value, parser):
         default=False,
         help="""\
 Output individual echo time series with slice, motion and susceptibility correction. \
-Useful for further Tedana processing post-fMRIPrep."""
+Useful for further Tedana processing post-fMRIPrep.""",
     )
 
     g_conf.add_argument(
@@ -370,9 +365,9 @@ def _slice_time_ref(value, parser):
         default=None,
         type=SliceTimeRef,
         help="The time of the reference slice to correct BOLD values to, as a fraction "
-             "acquisition time. 0 indicates the start, 0.5 the midpoint, and 1 the end "
-             "of acquisition. The alias `start` corresponds to 0, and `middle` to 0.5. "
-             "The default value is 0.5.",
+        "acquisition time. 0 indicates the start, 0.5 the midpoint, and 1 the end "
+        "of acquisition. The alias `start` corresponds to 0, and `middle` to 0.5. "
+        "The default value is 0.5.",
     )
     g_conf.add_argument(
         "--dummy-scans",
@@ -438,8 +433,7 @@ def _slice_time_ref(value, parser):
         action="store",
         default=1.5,
         type=float,
-        help="Threshold for flagging a frame as an outlier on the basis of standardised "
-        "DVARS",
+        help="Threshold for flagging a frame as an outlier on the basis of standardised " "DVARS",
     )
 
     #  ANTs options
@@ -499,7 +493,7 @@ def _slice_time_ref(value, parser):
         const="error",
         default=False,
         help="EXPERIMENTAL: Use fieldmap-free distortion correction; "
-             "if unable, error (default) or warn based on optional argument.",
+        "if unable, error (default) or warn based on optional argument.",
     )
     g_syn.add_argument(
         "--force-syn",
@@ -562,7 +556,7 @@ def _slice_time_ref(value, parser):
         help="Organization of outputs. bids (default) places fMRIPrep derivatives "
         "directly in the output directory, and defaults to placing FreeSurfer "
         "derivatives in <output-dir>/sourcedata/freesurfer. legacy creates "
-        "derivative datasets as subdirectories of outputs."
+        "derivative datasets as subdirectories of outputs.",
     )
     g_other.add_argument(
         "-w",
@@ -597,7 +591,8 @@ def _slice_time_ref(value, parser):
         action="store",
         metavar="FILE",
         help="Use pre-generated configuration file. Values in file will be overridden "
-             "by command-line arguments.")
+        "by command-line arguments.",
+    )
     g_other.add_argument(
         "--write-graph",
         action="store_true",
@@ -608,8 +603,7 @@ def _slice_time_ref(value, parser):
         "--stop-on-first-crash",
         action="store_true",
         default=False,
-        help="Force stopping on first crash, even if a work directory"
-        " was specified.",
+        help="Force stopping on first crash, even if a work directory" " was specified.",
     )
     g_other.add_argument(
         "--notrack",
@@ -665,6 +659,7 @@ def _slice_time_ref(value, parser):
 def parse_args(args=None, namespace=None):
     """Parse args and run further checks on the command line."""
     import logging
+
     from niworkflows.utils.spaces import Reference, SpatialReferences
 
     parser = _build_parser()
@@ -680,6 +675,7 @@ def parse_args(args=None, namespace=None):
 
     if not config.execution.notrack:
         import pkgutil
+
         if pkgutil.find_loader("sentry_sdk") is None:
             config.execution.notrack = True
             config.loggers.cli.warning("Telemetry disabled because sentry_sdk is not installed.")
@@ -759,9 +755,7 @@ def parse_args(args=None, namespace=None):
 
         build_log.info(f"Clearing previous fMRIPrep working directory: {work_dir}")
         if not clean_directory(work_dir):
-            build_log.warning(
-                f"Could not clear all contents of working directory: {work_dir}"
-            )
+            build_log.warning(f"Could not clear all contents of working directory: {work_dir}")
 
     # Update the config with an empty dict to trigger initialization of all config
     # sections (we used `init=False` above).
@@ -793,9 +787,7 @@ def parse_args(args=None, namespace=None):
             "Making sure the input data is BIDS compliant (warnings can be ignored in most "
             "cases)."
         )
-        validate_input_dir(
-            config.environment.exec_env, opts.bids_dir, opts.participant_label
-        )
+        validate_input_dir(config.environment.exec_env, opts.bids_dir, opts.participant_label)
 
     # Setup directories
     config.execution.log_dir = config.execution.fmriprep_dir / "logs"