MAINT: add (optional) pre-commit hook (scipy#17812)

Author: stefanv

HACKING.rst.txt

@@ -82,13 +82,22 @@ tests, benchmarks, and correct code style.
     as it will appear online.
 
 4. Code style
-    Uniformity of style in which code is written is important to others trying
-    to understand the code.  SciPy follows the standard Python guidelines for
-    code style, `PEP8`_.  In order to check that your code conforms to PEP8,
-    you can use the `pep8 package`_ style checker.  Most IDEs and text editors
-    have settings that can help you follow PEP8, for example by translating
-    tabs by four spaces.  Using `pyflakes`_ to check your code is also a good
-    idea. More information is available in :ref:`pep8-scipy`.
+    Uniform code style makes it easier for others to read your code.
+    SciPy follows the standard Python style guideline, `PEP8`_.
+
+    We provide a git pre-commit hook that can check each of your commits
+    for proper style. Install it (once) by running the following from
+    the root of the SciPy repository::
+
+      cp tools/pre-commit-hook.sh .git/hooks/pre-commit
+
+    Alternatively, you may run the linter manually::
+
+      python dev.py lint
+
+    Most IDEs and text editors also have settings that can help you
+    follow PEP8, for example by translating tabs by four spaces. More
+    information is available in :ref:`pep8-scipy`.
 
 A :ref:`checklist<pr-checklist>`, including these and other requirements, is
 available at the end of the example :ref:`development-workflow`.
@@ -259,8 +268,6 @@ improvements, and submit your first PR!
 
 .. _pep8 package: https://pypi.python.org/pypi/pep8
 
-.. _pyflakes: https://pypi.python.org/pypi/pyflakes
-
 .. _Github help pages: https://help.github.com/articles/set-up-git/
 
 .. _issue list: https://github.com/scipy/scipy/issues

azure-pipelines.yml

@@ -144,19 +144,17 @@ stages:
         addToPath: true
         architecture: 'x64'
     - script: >-
-        python -m pip install
-        "flake8<6"
+        python -m pip install ruff cython-lint
       displayName: 'Install tools'
       failOnStderr: false
     - script: |
         set -euo pipefail
-        flake8 scipy benchmarks/benchmarks
         # for Travis CI we used to do: git fetch --unshallow
         # but apparently Azure clones are not as shallow
         # so does not appear to be needed to fetch maintenance
         # branches (which Azure logs show being checked out
         # in Checkout stage)
-        python tools/lint_diff.py --branch origin/$(System.PullRequest.TargetBranch)
+        python tools/lint.py --diff-against origin/$(System.PullRequest.TargetBranch)
         tools/unicode-check.py
         tools/check_test_name.py
       displayName: 'Run Lint Checks'

dev.py

@@ -123,7 +123,6 @@ def task_meta(cls, **kwargs):
 
 import click
 from click import Option, Argument
-from doit import task_params
 from doit.cmd_base import ModuleTaskLoader
 from doit.reporter import ZeroReporter
 from doit.exceptions import TaskError
@@ -269,7 +268,7 @@ def cli(ctx, **kwargs):
 
     \b**python dev.py --build-dir my-build test -s stats**
 
-    """
+    """  # noqa: E501
     CLI.update_context(ctx, kwargs)
 
 
@@ -526,7 +525,7 @@ def install_project(cls, dirs, args):
                         log_size = os.stat(log_filename).st_size
                         if log_size > last_log_size:
                             elapsed = datetime.datetime.now() - start_time
-                            print("    ... installation in progress ({0} "
+                            print("    ... installation in progress ({} "
                                   "elapsed)".format(elapsed))
                             last_blip = time.time()
                             last_log_size = log_size
@@ -539,7 +538,7 @@ def install_project(cls, dirs, args):
 
         if ret != 0:
             if not args.show_build_log:
-                with open(log_filename, 'r') as f:
+                with open(log_filename) as f:
                     print(f.read())
             print(f"Installation failed! ({elapsed} elapsed)")
             sys.exit(1)
@@ -558,7 +557,7 @@ def copy_openblas(cls, dirs):
         default `_distributor_init.py` file with the one
         we use for wheels uploaded to PyPI so that DLL gets loaded.
 
-        Assumes pkg-config is installed and aware of OpenBLAS. 
+        Assumes pkg-config is installed and aware of OpenBLAS.
 
         The "dirs" parameter is typically a "Dirs" object with the
         structure as the following, say, if dev.py is run from the
@@ -652,7 +651,7 @@ class Test(Task):
     $ python dev.py test -s cluster -m full --durations 20
     $ python dev.py test -s stats -- --tb=line  # `--` passes next args to pytest
     ```
-    """
+    """  # noqa: E501
     ctx = CONTEXT
 
     verbose = Option(
@@ -889,29 +888,14 @@ def emit_cmdstr(cmd):
     console.print(f"{EMOJI.cmd} [cmd] {cmd}")
 
 
-@task_params([{'name': 'output_file', 'long': 'output-file', 'default': None,
-               'help': 'Redirect report to a file'}])
-def task_flake8(output_file):
-    """Run flake8 over the code base and benchmarks."""
-    opts = ''
-    if output_file:
-        opts += f'--output-file={output_file}'
-
-    cmd = f"flake8 {opts} scipy benchmarks/benchmarks"
-    emit_cmdstr(f"{cmd}")
-    return {
-        'actions': [cmd],
-        'doc': 'Lint scipy and benchmarks directory',
-    }
-
-
-def task_pep8diff():
+def task_lint():
     # Lint just the diff since branching off of main using a
     # stricter configuration.
-    emit_cmdstr(os.path.join('tools', 'lint_diff.py'))
+    emit_cmdstr(os.path.join('tools', 'lint.py') + ' --diff-against main')
     return {
-        'basename': 'pep8-diff',
-        'actions': [str(Dirs().root / 'tools' / 'lint_diff.py')],
+        'basename': 'lint',
+        'actions': [str(Dirs().root / 'tools' / 'lint.py') +
+                    ' --diff-against=main'],
         'doc': 'Lint only files modified since last commit (stricter rules)',
     }
 
@@ -937,16 +921,11 @@ def task_check_test_name():
 
 @cli.cls_cmd('lint')
 class Lint():
-    """:dash: Run flake8, check PEP 8 compliance on branch diff and check for
+    """:dash: Run linter on modified files and check for
     disallowed Unicode characters and possibly-invalid test names."""
-    output_file = Option(
-        ['--output-file'], default=None, help='Redirect report to a file')
-
-    def run(output_file):
-        opts = {'output_file': output_file}
+    def run():
         run_doit_task({
-            'flake8': opts,
-            'pep8-diff': {},
+            'lint': {},
             'unicode-check': {},
             'check-testname': {},
         })
@@ -1109,7 +1088,7 @@ def run(cls, pythonpath, extra_argv=None, **kwargs):
             # Don't use subprocess, since we don't want to include the
             # current path in PYTHONPATH.
             sys.argv = extra_argv
-            with open(extra_argv[0], 'r') as f:
+            with open(extra_argv[0]) as f:
                 script = f.read()
             sys.modules['__main__'] = new_module('__main__')
             ns = dict(__name__='__main__', __file__=extra_argv[0])

doc/source/dev/contributor/pep8.rst

@@ -19,30 +19,32 @@ compliance before pushing your code:
    Editor |rarr| Advanced Settings. This can help you fix PEP8 issues as you
    write your code.
 
--  SciPy makes use of special configuration files for linting with the
-   |flake8|_ tool.
+-  Note, however, that SciPy's linting configuration may not match
+   that of your IDE exactly. See below on how to run the official
+   checks.
 
--  It is typically recommended to leave any existing style issues alone
-   unless they are part of the code you're already modifying.
+-  It is recommended to leave existing style issues alone
+   unless they exist in files you are already modifying.
    This practice ensures that the codebase is gradually cleaned up
    without dedicating precious review time to style-only cleanups.
-   Before sending a Pull Request, we suggest running the lint tests only
-   for the changes you've made in your feature branch. This will mimic
-   the continuous integration linting checks setup on GitHub.
-   After installing ``flake8``, you can run the following check locally
-   in the SciPy root directory to ensure your Pull Request doesn't
-   break the Continuous Integration linting tests.
 
-   ::
+-  Before sending a Pull Request, run the linter on changes made in
+   your feature branch. The checks will also be made during
+   continuous integration, but it's quicker to catch them early.
 
-      python tools/lint_diff.py
+   The easiest way to do so is to install our pre-commit hook (once)::
 
-   If you want to run the diff based lint tests only for specific files
-   or directories, please consider using the ``--files`` option.
+     cp tools/pre-commit-hook.sh .git/hooks/pre-commit
 
-   ::
+   This will run linting checks before each commit is made.
 
-      python tools/lint_diff.py --files scipy/odr/models.py scipy/ndimage
+   Alternatively, you can run the check manually from the SciPy root directory::
+
+      python dev.py lint
+
+   You can also run the linter on specific files, using the ``--files`` option::
+
+      python tools/lint.py --files scipy/odr/models.py scipy/ndimage
 
 -  If you have existing code with a lot of PEP8 issues, consider using
    |autopep8|_ to automatically fix them before incorporating the code into
@@ -51,9 +53,6 @@ compliance before pushing your code:
 .. _PEP8: https://www.python.org/dev/peps/pep-0008/
 .. _enable Real-time code style analysis: https://stackoverflow.com/questions/51463223/how-to-use-pep8-module-using-spyder
 
-.. |flake8| replace:: ``flake8``
-.. _flake8: http://flake8.pycqa.org/en/latest/
-
 .. |autopep8| replace:: ``autopep8``
 .. _autopep8: https://pypi.org/project/autopep8/0.8/
 

environment.yml

@@ -39,7 +39,6 @@ dependencies:
   - pydata-sphinx-theme==0.9.0
   - sphinx-design
   # For linting
-  - flake8 < 6.0
   # Some optional test dependencies
   - mpmath
   - gmpy2
@@ -49,3 +48,7 @@ dependencies:
   - click
   - doit>=0.36.0
   - pydevtool
+  - pip
+  - pip:
+    - ruff
+    - cython-lint

pyproject.toml

@@ -121,7 +121,8 @@ dev = [
     "mypy",
     "typing_extensions",
     "pycodestyle",
-    "flake8",
+    "ruff",
+    "cython-lint",
     "rich-click",
     "click",
     "doit>=0.36.0",

runtests.py

@@ -39,22 +39,23 @@
 EXTRA_PATH = ['/usr/lib/ccache', '/usr/lib/f90cache',
               '/usr/local/lib/ccache', '/usr/local/lib/f90cache']
 
-# ---------------------------------------------------------------------
-
 
 if __doc__ is None:
     __doc__ = "Run without -OO if you want usage info"
 else:
     __doc__ = __doc__.format(**globals())
 
 
+# ---------------------------------------------------------------------
+# ruff: noqa E402
+
 import sys
 import os
 import errno
 # the following multiprocessing import is necessary to prevent tests that use
 # multiprocessing from hanging on >= Python3.8 (macOS) using pytest. Just the
 # import is enough...
-import multiprocessing
+import multiprocessing  # noqa: F401
 
 
 # In case we are run from the source directory, we don't want to import the
@@ -69,6 +70,7 @@
 import datetime
 from types import ModuleType as new_module  # noqa: E402
 
+
 ROOT_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__)))
 
 
@@ -128,20 +130,16 @@ def main(argv):
     parser.add_argument("args", metavar="ARGS", default=[], nargs=REMAINDER,
                         help="Arguments to pass to Nose, Python or shell")
     parser.add_argument("--pep8", action="store_true", default=False,
-                        help="Perform pep8 check with flake8.")
+                        help="Perform pep8 check.")
     parser.add_argument("--mypy", action="store_true", default=False,
                         help="Run mypy on the codebase")
     parser.add_argument("--doc", action="append", nargs="?",
                         const="html", help="Build documentation")
     args = parser.parse_args(argv)
 
     if args.pep8:
-        # Lint the source using the configuration in tox.ini.
-        os.system("flake8 scipy benchmarks/benchmarks")
-        # Lint just the diff since branching off of main using a
-        # stricter configuration.
-        lint_diff = os.path.join(ROOT_DIR, 'tools', 'lint_diff.py')
-        os.system(lint_diff)
+        linter = os.path.join(ROOT_DIR, 'tools', 'lint.py')
+        os.system(linter + " --diff-against=main")
         sys.exit(0)
 
     if args.mypy: