diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 7c108f7eb2c..dcc9ecb3cec 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -37,7 +37,14 @@ jobs: id: deps run: pip install tox - - name: Code style check with pycodestyle + - name: Code style check with ruff-minimal + if: (success() || failure()) && steps.deps.outcome == 'success' + run: tox -e ruff-minimal + env: + # https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308 + RUFF_OUTPUT_FORMAT: github + + - name: Code style check with pycodestyle-minimal if: (success() || failure()) && steps.deps.outcome == 'success' run: tox -e pycodestyle-minimal diff --git a/ruff.toml b/ruff.toml index d6dd6fed1bd..b3070914153 100644 --- a/ruff.toml +++ b/ruff.toml @@ -3,12 +3,12 @@ # Assume Python 3.9 target-version = "py39" -select = [ +lint.select = [ "E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e "F", # pyflakes - https://docs.astral.sh/ruff/rules/#pyflakes-f "I", # isort - https://docs.astral.sh/ruff/rules/#isort-i "PL", # pylint - https://docs.astral.sh/ruff/rules/#pylint-pl ] -ignore = [ +lint.ignore = [ "E501", # Line too long - hard to avoid in doctests, and better handled by black. ] diff --git a/src/doc/en/developer/tools.rst b/src/doc/en/developer/tools.rst index 737d7410a48..bc18723cb24 100644 --- a/src/doc/en/developer/tools.rst +++ b/src/doc/en/developer/tools.rst @@ -49,7 +49,7 @@ available:: --tox [options] -- general entry point for testing and linting of the Sage library -e -- run specific test environments; default: - doctest,coverage,startuptime,pycodestyle-minimal,relint,codespell,rst + doctest,coverage,startuptime,pycodestyle-minimal,relint,codespell,rst,ruff-minimal doctest -- run the Sage doctester (same as "sage -t") coverage -- give information about doctest coverage of files @@ -60,11 +60,13 @@ available:: relint -- check whether some forbidden patterns appear codespell -- check for misspelled words in source code rst -- validate Python docstrings markup as reStructuredText + ruff-minimal -- check against Sage's minimal style conventions coverage.py -- run the Sage doctester with Coverage.py coverage.py-html -- run the Sage doctester with Coverage.py, generate HTML report pyright -- run the static typing checker pyright pycodestyle -- check against the Python style conventions of PEP8 cython-lint -- check Cython files for code style + ruff -- check against Python style conventions -p auto -- run test environments in parallel --help -- show tox help @@ -287,6 +289,20 @@ for Python code, written in Rust. It comes with a large choice of possible checks, and has the capacity to fix some of the warnings it emits. +Sage defines two configurations for ruff. The command ``./sage -tox -e ruff-minimal`` uses +ruff in a minimal configuration. As of Sage 10.3, the entire Sage library conforms to this +configuration. When preparing a Sage PR, developers should verify that +``./sage -tox -e ruff-minimal`` passes. + +The second configuration is used with the command ``./sage -tox -e ruff`` and runs a +more thorough check. When preparing a PR that adds new code, +developers should verify that ``./sage -tox -e ruff`` does not +issue warnings for the added code. This will avoid later cleanup +PRs as the Sage codebase is moving toward full PEP 8 compliance. + +On the other hand, it is usually not advisable to mix coding-style +fixes with productive changes on the same PR because this would +makes it harder for reviewers to evaluate the changes. .. _section-tools-relint: diff --git a/src/tox.ini b/src/tox.ini index ca69ed6c8bf..6e6a084b3db 100644 --- a/src/tox.ini +++ b/src/tox.ini @@ -21,7 +21,7 @@ ## in a virtual environment. ## [tox] -envlist = doctest, coverage, startuptime, pycodestyle-minimal, relint, codespell, rst +envlist = doctest, coverage, startuptime, pycodestyle-minimal, relint, codespell, rst, ruff-minimal # When adding environments above, also update the delegations in SAGE_ROOT/tox.ini skipsdist = true @@ -259,6 +259,66 @@ description = deps = cython-lint commands = cython-lint --no-pycodestyle {posargs:{toxinidir}/sage/} +[testenv:ruff] +description = + check against Python style conventions +deps = ruff +passenv = RUFF_OUTPUT_FORMAT +commands = ruff check {posargs:{toxinidir}/sage/} + +[testenv:ruff-minimal] +description = + check against Sage's minimal style conventions +deps = ruff +# https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308 +passenv = RUFF_OUTPUT_FORMAT +# Output of currently failing, from "./sage -tox -e ruff -- --statistics": +# +# 3579 PLR2004 [ ] Magic value used in comparison, consider replacing `- 0.5` with a constant variable +# 3498 I001 [*] Import block is un-sorted or un-formatted +# 2146 F401 [*] `.PyPolyBoRi.Monomial` imported but unused +# 1964 E741 [ ] Ambiguous variable name: `I` +# 1676 F821 [ ] Undefined name `AA` +# 1513 PLR0912 [ ] Too many branches (102 > 12) +# 1159 PLR0913 [ ] Too many arguments in function definition (10 > 5) +# 815 E402 [ ] Module level import not at top of file +# 671 PLR0915 [ ] Too many statements (100 > 50) +# 483 PLW2901 [ ] Outer `for` loop variable `ext` overwritten by inner `for` loop target +# 433 PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation +# 428 PLR0911 [ ] Too many return statements (10 > 6) +# 404 E731 [*] Do not assign a `lambda` expression, use a `def` +# 351 F405 [ ] `ComplexField` may be undefined, or defined from star imports +# 306 PLR1714 [*] Consider merging multiple comparisons. Use a `set` if the elements are hashable. +# 236 F403 [ ] `from .abelian_gps.all import *` used; unable to detect undefined names +# 116 PLR0402 [*] Use `from matplotlib import cm` in lieu of alias +# 111 PLW0603 [ ] Using the global statement to update `AA_0` is discouraged +# 78 F841 [*] Local variable `B` is assigned to but never used +# 64 E713 [*] Test for membership should be `not in` +# 48 PLW0602 [ ] Using global for `D` but no assignment is done +# 33 PLR1711 [*] Useless `return` statement at end of function +# 24 E714 [*] Test for object identity should be `is not` +# 20 PLR1701 [*] Merge `isinstance` calls +# 17 PLW3301 [ ] Nested `max` calls can be flattened +# 16 PLW1510 [*] `subprocess.run` without explicit `check` argument +# 14 E721 [ ] Do not compare types, use `isinstance()` +# 14 PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and dedent its contents +# 12 F811 [*] Redefinition of unused `CompleteDiscreteValuationRings` from line 49 +# 8 PLC0414 [*] Import alias does not rename original package +# 7 E743 [ ] Ambiguous function name: `I` +# 7 PLE0101 [ ] Explicit return in `__init__` +# 7 PLR0124 [ ] Name compared with itself, consider replacing `a == a` +# 5 PLW0127 [ ] Self-assignment of variable `a` +# 4 F541 [*] f-string without any placeholders +# 4 PLW1508 [ ] Invalid type for environment variable default; expected `str` or `None` +# 3 PLC3002 [ ] Lambda expression called directly. Execute the expression inline instead. +# 2 E742 [ ] Ambiguous class name: `I` +# 2 PLE0302 [ ] The special method `__len__` expects 1 parameter, 3 were given +# 2 PLW0129 [ ] Asserting on a non-empty string literal will always pass +# 1 F402 [ ] Import `factor` from line 259 shadowed by loop variable +# 1 PLC0208 [*] Use a sequence type instead of a `set` when iterating over values +# +commands = ruff check --ignore PLR2004,I001,F401,E741,F821,PLR0912,PLR0913,E402,PLR0915,PLW2901,PLR5501,PLR0911,E731,F405,PLR1714,F403,PLR0402,PLW0603,F841,E713,PLW0602,PLR1711,E714,PLR1701,PLW3301,PLW1510,E721,PLW0120,F811,PLC0414,E743,PLE0101,PLR0124,PLW0127,F541,PLW1508,PLC3002,E742,PLE0302,PLW0129,F402,PLC0208 {posargs:{toxinidir}/sage/} + [flake8] rst-roles = # Sphinx diff --git a/tox.ini b/tox.ini index b06f4529afc..4a5d4fb7834 100644 --- a/tox.ini +++ b/tox.ini @@ -1090,3 +1090,23 @@ passenv = {[sage_src]passenv} envdir = {[sage_src]envdir} commands = {[sage_src]commands} allowlist_externals = {[sage_src]allowlist_externals} + +[testenv:ruff] +description = + check against Python style conventions +passenv = {[sage_src]passenv} + # https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308 + RUFF_OUTPUT_FORMAT +envdir = {[sage_src]envdir} +allowlist_externals = {[sage_src]allowlist_externals} +commands = tox -c {toxinidir}/src/tox.ini -e {envname} -- {posargs:src/sage/} + +[testenv:ruff-minimal] +description = + check against Sage's minimal style conventions +passenv = {[sage_src]passenv} + # https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308 + RUFF_OUTPUT_FORMAT +envdir = {[sage_src]envdir} +allowlist_externals = {[sage_src]allowlist_externals} +commands = tox -c {toxinidir}/src/tox.ini -e {envname} -- {posargs:src/sage/}