diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index c9b3aeb71a6..40b6f52c00a 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -22,13 +22,12 @@ "vscode": { "extensions": [ "guyskk.language-cython", - "ms-python.isort", "ms-toolsai.jupyter", "ms-python.vscode-pylance", - "ms-python.pylint", "ms-python.python", "lextudio.restructuredtext", - "trond-snekvik.simple-rst" + "trond-snekvik.simple-rst", + "charliermarsh.ruff" ] } } diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 5d444594877..18bfdc167c7 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/.vscode/extensions.json b/.vscode/extensions.json index d0efd087bc9..a304fa28467 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -2,6 +2,7 @@ // List of extensions which should be recommended for developers when a workspace is opened for the first time. // See https://go.microsoft.com/fwlink/?LinkId=827846 to learn about workspace recommendations. "recommendations": [ - "ms-python.python" + "ms-python.python", + "charliermarsh.ruff" ], } diff --git a/.vscode/settings.json b/.vscode/settings.json index 86885e6ea4a..86eac03ffe9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -24,11 +24,6 @@ "--doctest-modules" ], "python.testing.unittestEnabled": false, - "python.linting.pycodestyleEnabled": true, - "python.linting.enabled": true, - // The following pycodestyle arguments are the same as the pycodestyle-minimal - // tox environnment, see the file SAGE_ROOT/src/tox.ini - "python.linting.pycodestyleArgs": ["--select= E111,E21,E222,E225,E227,E228,E25,E271,E303,E305,E306,E401,E502,E701,E702,E703,E71,E72,W291,W293,W391,W605"], "cSpell.words": [ "furo", "Conda", diff --git a/src/doc/en/developer/tools.rst b/src/doc/en/developer/tools.rst index 02be3f91b4b..fdb93020121 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 @@ -61,11 +61,13 @@ available:: (includes all patchbot pattern-exclusion plugins) 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 @@ -288,6 +290,21 @@ 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: Relint diff --git a/src/ruff.toml b/src/ruff.toml new file mode 100644 index 00000000000..b3070914153 --- /dev/null +++ b/src/ruff.toml @@ -0,0 +1,14 @@ +# https://docs.astral.sh/ruff/configuration/#config-file-discovery + +# Assume Python 3.9 +target-version = "py39" + +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 +] +lint.ignore = [ + "E501", # Line too long - hard to avoid in doctests, and better handled by black. +] diff --git a/src/tox.ini b/src/tox.ini index 8e562cced8e..2c0364869c6 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 @@ -240,6 +240,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 {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 -- check --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 --ignore I001,PLR2004,F401,E741,F821,PLR0912,PLR0913,E402,PLR0915,PLW2901,PLR5501,PLR0911,E731,F405,PLR1714,F403,PLR0402,PLW0603,E713,F841,PLW0602,E714,PLR1711,PLR1701,PLW3301,E721,PLW1510,F811,PLW0120,PLC0414,E743,F541,PLE0101,PLR0124,PLW0127,PLW1508,PLC3002,E742,PLE0302,F402,PLC0208 {posargs:{toxinidir}/sage/} + [flake8] rst-roles = # Sphinx diff --git a/tox.ini b/tox.ini index 3673833724c..fc932501597 100644 --- a/tox.ini +++ b/tox.ini @@ -1045,3 +1045,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/}