Skip to content

Commit

Permalink
MAINT: add (optional) pre-commit hook (#17812)
Browse files Browse the repository at this point in the history
  • Loading branch information
stefanv committed Jan 29, 2023
1 parent 03c337a commit 146bf0e
Show file tree
Hide file tree
Showing 14 changed files with 292 additions and 202 deletions.
25 changes: 16 additions & 9 deletions HACKING.rst.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
49 changes: 14 additions & 35 deletions dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)


Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)',
}

Expand All @@ -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': {},
})
Expand Down Expand Up @@ -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])
Expand Down
37 changes: 18 additions & 19 deletions doc/source/dev/contributor/pep8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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/

Expand Down
5 changes: 4 additions & 1 deletion environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ dependencies:
- pydata-sphinx-theme==0.9.0
- sphinx-design
# For linting
- flake8 < 6.0
# Some optional test dependencies
- mpmath
- gmpy2
Expand All @@ -49,3 +48,7 @@ dependencies:
- click
- doit>=0.36.0
- pydevtool
- pip
- pip:
- ruff
- cython-lint
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ dev = [
"mypy",
"typing_extensions",
"pycodestyle",
"flake8",
"ruff",
"cython-lint",
"rich-click",
"click",
"doit>=0.36.0",
Expand Down
18 changes: 8 additions & 10 deletions runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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__)))


Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 146bf0e

Please sign in to comment.