Skip to content

Commit 56d61ca

Browse files
author
Release Manager
committed
sagemathgh-37453: tox.ini: Add environments `ruff`, `ruff-minimal`; GH Actions: run `ruff-minimal` <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> `./sage -tox` and the GH Actions "Lint" workflow now additionally run `ruff-minimal`. The "Lint" workflow outputs GitHub annotations for view in the "Files changed" tab, as pioneered in sagemath#36512 (see screenshot there). Demo: https://github.com/sagemath/sage/pull/37456/files We use the built-in capability of ruff to output via `RUFF_OUTPUT_FORMAT=github` (no problem matcher is needed; see https://github.com/ChartBoost/ruff- action/issues/7#issuecomment-1887780308). (This has been adopted in the revised sagemath#36512.) sagemath#36512 (comment) is marked "disputed" because it builds upon the sagemath#36503, which bundles a controversial design choice, as explained in sagemath#37452. In further contrast to sagemath#36512, we do not remove the pycodestyle-minimal run from the "Lint" workflow. This can be done in a follow-up, once we have gained the necessary experience with the new linter (see previous info request in sagemath#36512 (comment)). Hence I am marking sagemath#36512 not as a "duplicate" but as "pending"; and "disputed" because of its dependency on the "disputed" sagemath#36503. @roed314 @vbraun And in further contrast to sagemath#36512, the minimal ruff configuration used by the CI can be used locally with `./sage -tox -e ruff-minimal` and also runs as part of the default tests in `./sage -tox`. Authors: @mkoeppe, @tobiasdiez (credit for the first version of the minimal ruff configuration taken from sagemath#36512, now regenerated) <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> - Depends on sagemath#37452 <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#37453 Reported by: Matthias Köppe Reviewer(s): Frédéric Chapoton, Kwankyu Lee, Matthias Köppe
2 parents dc67af1 + c97451d commit 56d61ca

File tree

5 files changed

+108
-5
lines changed

5 files changed

+108
-5
lines changed

.github/workflows/lint.yml

+8-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,14 @@ jobs:
3737
id: deps
3838
run: pip install tox
3939

40-
- name: Code style check with pycodestyle
40+
- name: Code style check with ruff-minimal
41+
if: (success() || failure()) && steps.deps.outcome == 'success'
42+
run: tox -e ruff-minimal
43+
env:
44+
# https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308
45+
RUFF_OUTPUT_FORMAT: github
46+
47+
- name: Code style check with pycodestyle-minimal
4148
if: (success() || failure()) && steps.deps.outcome == 'success'
4249
run: tox -e pycodestyle-minimal
4350

ruff.toml

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@
33
# Assume Python 3.9
44
target-version = "py39"
55

6-
select = [
6+
lint.select = [
77
"E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e
88
"F", # pyflakes - https://docs.astral.sh/ruff/rules/#pyflakes-f
99
"I", # isort - https://docs.astral.sh/ruff/rules/#isort-i
1010
"PL", # pylint - https://docs.astral.sh/ruff/rules/#pylint-pl
1111
]
12-
ignore = [
12+
lint.ignore = [
1313
"E501", # Line too long - hard to avoid in doctests, and better handled by black.
1414
]

src/doc/en/developer/tools.rst

+17-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ available::
4949
--tox [options] <files|dirs> -- general entry point for testing
5050
and linting of the Sage library
5151
-e <envlist> -- run specific test environments; default:
52-
doctest,coverage,startuptime,pycodestyle-minimal,relint,codespell,rst
52+
doctest,coverage,startuptime,pycodestyle-minimal,relint,codespell,rst,ruff-minimal
5353
doctest -- run the Sage doctester
5454
(same as "sage -t")
5555
coverage -- give information about doctest coverage of files
@@ -60,11 +60,13 @@ available::
6060
relint -- check whether some forbidden patterns appear
6161
codespell -- check for misspelled words in source code
6262
rst -- validate Python docstrings markup as reStructuredText
63+
ruff-minimal -- check against Sage's minimal style conventions
6364
coverage.py -- run the Sage doctester with Coverage.py
6465
coverage.py-html -- run the Sage doctester with Coverage.py, generate HTML report
6566
pyright -- run the static typing checker pyright
6667
pycodestyle -- check against the Python style conventions of PEP8
6768
cython-lint -- check Cython files for code style
69+
ruff -- check against Python style conventions
6870
-p auto -- run test environments in parallel
6971
--help -- show tox help
7072

@@ -287,6 +289,20 @@ for Python code, written in Rust.
287289
It comes with a large choice of possible checks, and has the capacity
288290
to fix some of the warnings it emits.
289291

292+
Sage defines two configurations for ruff. The command ``./sage -tox -e ruff-minimal`` uses
293+
ruff in a minimal configuration. As of Sage 10.3, the entire Sage library conforms to this
294+
configuration. When preparing a Sage PR, developers should verify that
295+
``./sage -tox -e ruff-minimal`` passes.
296+
297+
The second configuration is used with the command ``./sage -tox -e ruff`` and runs a
298+
more thorough check. When preparing a PR that adds new code,
299+
developers should verify that ``./sage -tox -e ruff`` does not
300+
issue warnings for the added code. This will avoid later cleanup
301+
PRs as the Sage codebase is moving toward full PEP 8 compliance.
302+
303+
On the other hand, it is usually not advisable to mix coding-style
304+
fixes with productive changes on the same PR because this would
305+
makes it harder for reviewers to evaluate the changes.
290306

291307
.. _section-tools-relint:
292308

src/tox.ini

+61-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
## in a virtual environment.
2222
##
2323
[tox]
24-
envlist = doctest, coverage, startuptime, pycodestyle-minimal, relint, codespell, rst
24+
envlist = doctest, coverage, startuptime, pycodestyle-minimal, relint, codespell, rst, ruff-minimal
2525
# When adding environments above, also update the delegations in SAGE_ROOT/tox.ini
2626
skipsdist = true
2727

@@ -259,6 +259,66 @@ description =
259259
deps = cython-lint
260260
commands = cython-lint --no-pycodestyle {posargs:{toxinidir}/sage/}
261261
262+
[testenv:ruff]
263+
description =
264+
check against Python style conventions
265+
deps = ruff
266+
passenv = RUFF_OUTPUT_FORMAT
267+
commands = ruff check {posargs:{toxinidir}/sage/}
268+
269+
[testenv:ruff-minimal]
270+
description =
271+
check against Sage's minimal style conventions
272+
deps = ruff
273+
# https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308
274+
passenv = RUFF_OUTPUT_FORMAT
275+
# Output of currently failing, from "./sage -tox -e ruff -- --statistics":
276+
#
277+
# 3579 PLR2004 [ ] Magic value used in comparison, consider replacing `- 0.5` with a constant variable
278+
# 3498 I001 [*] Import block is un-sorted or un-formatted
279+
# 2146 F401 [*] `.PyPolyBoRi.Monomial` imported but unused
280+
# 1964 E741 [ ] Ambiguous variable name: `I`
281+
# 1676 F821 [ ] Undefined name `AA`
282+
# 1513 PLR0912 [ ] Too many branches (102 > 12)
283+
# 1159 PLR0913 [ ] Too many arguments in function definition (10 > 5)
284+
# 815 E402 [ ] Module level import not at top of file
285+
# 671 PLR0915 [ ] Too many statements (100 > 50)
286+
# 483 PLW2901 [ ] Outer `for` loop variable `ext` overwritten by inner `for` loop target
287+
# 433 PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
288+
# 428 PLR0911 [ ] Too many return statements (10 > 6)
289+
# 404 E731 [*] Do not assign a `lambda` expression, use a `def`
290+
# 351 F405 [ ] `ComplexField` may be undefined, or defined from star imports
291+
# 306 PLR1714 [*] Consider merging multiple comparisons. Use a `set` if the elements are hashable.
292+
# 236 F403 [ ] `from .abelian_gps.all import *` used; unable to detect undefined names
293+
# 116 PLR0402 [*] Use `from matplotlib import cm` in lieu of alias
294+
# 111 PLW0603 [ ] Using the global statement to update `AA_0` is discouraged
295+
# 78 F841 [*] Local variable `B` is assigned to but never used
296+
# 64 E713 [*] Test for membership should be `not in`
297+
# 48 PLW0602 [ ] Using global for `D` but no assignment is done
298+
# 33 PLR1711 [*] Useless `return` statement at end of function
299+
# 24 E714 [*] Test for object identity should be `is not`
300+
# 20 PLR1701 [*] Merge `isinstance` calls
301+
# 17 PLW3301 [ ] Nested `max` calls can be flattened
302+
# 16 PLW1510 [*] `subprocess.run` without explicit `check` argument
303+
# 14 E721 [ ] Do not compare types, use `isinstance()`
304+
# 14 PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
305+
# 12 F811 [*] Redefinition of unused `CompleteDiscreteValuationRings` from line 49
306+
# 8 PLC0414 [*] Import alias does not rename original package
307+
# 7 E743 [ ] Ambiguous function name: `I`
308+
# 7 PLE0101 [ ] Explicit return in `__init__`
309+
# 7 PLR0124 [ ] Name compared with itself, consider replacing `a == a`
310+
# 5 PLW0127 [ ] Self-assignment of variable `a`
311+
# 4 F541 [*] f-string without any placeholders
312+
# 4 PLW1508 [ ] Invalid type for environment variable default; expected `str` or `None`
313+
# 3 PLC3002 [ ] Lambda expression called directly. Execute the expression inline instead.
314+
# 2 E742 [ ] Ambiguous class name: `I`
315+
# 2 PLE0302 [ ] The special method `__len__` expects 1 parameter, 3 were given
316+
# 2 PLW0129 [ ] Asserting on a non-empty string literal will always pass
317+
# 1 F402 [ ] Import `factor` from line 259 shadowed by loop variable
318+
# 1 PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
319+
#
320+
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/}
321+
262322
[flake8]
263323
rst-roles =
264324
# Sphinx

tox.ini

+20
Original file line numberDiff line numberDiff line change
@@ -1090,3 +1090,23 @@ passenv = {[sage_src]passenv}
10901090
envdir = {[sage_src]envdir}
10911091
commands = {[sage_src]commands}
10921092
allowlist_externals = {[sage_src]allowlist_externals}
1093+
1094+
[testenv:ruff]
1095+
description =
1096+
check against Python style conventions
1097+
passenv = {[sage_src]passenv}
1098+
# https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308
1099+
RUFF_OUTPUT_FORMAT
1100+
envdir = {[sage_src]envdir}
1101+
allowlist_externals = {[sage_src]allowlist_externals}
1102+
commands = tox -c {toxinidir}/src/tox.ini -e {envname} -- {posargs:src/sage/}
1103+
1104+
[testenv:ruff-minimal]
1105+
description =
1106+
check against Sage's minimal style conventions
1107+
passenv = {[sage_src]passenv}
1108+
# https://github.com/ChartBoost/ruff-action/issues/7#issuecomment-1887780308
1109+
RUFF_OUTPUT_FORMAT
1110+
envdir = {[sage_src]envdir}
1111+
allowlist_externals = {[sage_src]allowlist_externals}
1112+
commands = tox -c {toxinidir}/src/tox.ini -e {envname} -- {posargs:src/sage/}

0 commit comments

Comments
 (0)