Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tox -e ruff-minimal: Do not ignore PLW0129 #37456

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
}
Expand Down
9 changes: 8 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion .vscode/extensions.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
],
}
5 changes: 0 additions & 5 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 18 additions & 1 deletion src/doc/en/developer/tools.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ available::
--tox [options] <files|dirs> -- general entry point for testing
and linting of the Sage library
-e <envlist> -- 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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions src/ruff.toml
Original file line number Diff line number Diff line change
@@ -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.
]
62 changes: 61 additions & 1 deletion src/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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/}
Loading