From a99f822194bcedade868370fbf4f8d614f4fc610 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 14:01:31 -0700 Subject: [PATCH 01/18] Cleanup ruff configuration --- .github/workflows/lint.yml | 4 +- .github/workflows/ruff.toml | 14 ++++++ pyproject.toml | 27 +++++++++--- src/sage/coding/linear_code.py | 2 +- src/sage_docbuild/__main__.py | 4 +- src/sage_docbuild/builders.py | 2 +- src/tox.ini | 78 +++++++++++++++------------------- 7 files changed, 77 insertions(+), 54 deletions(-) create mode 100644 .github/workflows/ruff.toml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 94e87ccafdf..e841e430a4a 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -34,8 +34,8 @@ jobs: - name: Code style check with ruff-minimal if: (success() || failure()) && steps.deps.outcome == 'success' run: | - uv run --frozen --only-group lint -- ruff check --output-format github --ignore E402,E721,E731,E741,E742,E743,F401,F402,F403,F405,F821,F841,I001,PLC0206,PLC0208,PLC1802,PLC2401,PLC3002,PLC0415,PLE0302,PLR0124,PLR0402,PLR0911,PLR0912,PLR0913,PLR0915,PLR1704,PLR1711,PLR1714,PLR1716,PLR1733,PLR1736,PLR2004,PLR5501,PLW0120,PLW0211,PLW0602,PLW0603,PLW0642,PLW1508,PLW1510,PLW1641,PLW2901,PLW3301 src/ - uv run --frozen --only-group lint -- ruff check --output-format github --preview --select E111,E115,E21,E221,E222,E225,E227,E228,E25,E271,E272,E275,E302,E303,E305,E306,E401,E502,E701,E702,E703,E71,W291,W293,W391,W605,RUF013,RUF036,TC,UP004,UP006,UP008,UP010,UP015,UP022,UP034,UP035 src/sage/ + uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml + uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --preview - name: Code style check with relint if: (success() || failure()) && steps.deps.outcome == 'success' diff --git a/.github/workflows/ruff.toml b/.github/workflows/ruff.toml new file mode 100644 index 00000000000..800582df293 --- /dev/null +++ b/.github/workflows/ruff.toml @@ -0,0 +1,14 @@ +# Configuration for GitHub Action running ruff +extend = "../../pyproject.toml" # Start with the project configuration + +[lint] +# Remove rules that we do not yet follow across the whole codebase. +ignore = [ + "E402", "E721", "E731", "E741", "E742", "E743", + "F401", "F402", "F403", "F405", "F821", "F841", + "I001", + "PLC0206", "PLC0208", "PLC1802", "PLC2401", "PLC3002", "PLC0415", + "PLE0302", + "PLR0124", "PLR0402", "PLR0911", "PLR0912", "PLR0913", "PLR0915", "PLR1704", "PLR1711", "PLR1714", "PLR1716", "PLR1733", "PLR1736", "PLR2004", "PLR5501", + "PLW0120", "PLW0211", "PLW0602", "PLW0603", "PLW0642", "PLW1508", "PLW1510", "PLW1641", "PLW2901", "PLW3301", +] diff --git a/pyproject.toml b/pyproject.toml index 0c6ae3aa351..0b23479730e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -235,20 +235,37 @@ test = ["coverage", "pytest", "pytest-xdist"] # Python 3.12 is the minimum supported version target-version = "py312" -# This directory is full of old autogenerated files. -# Linting it would just get in the way of adding stricter linter rules -# to our actual code -exclude = ["src/doc/"] +exclude = [ + # This directory is full of old autogenerated files. + # Linting it would just get in the way of adding stricter linter rules + # to our actual code. + "src/doc/", + # This directory handles bootstrapping the build process, + # and allows Python versions as old as 2.6, so there is no point in linting it. + "build/sage_bootstrap/", +] [tool.ruff.lint] + +# Rules that we deliberately do not attempt to follow, either because we +# disagree with them or because following them is not currently practical. ignore = [ "E501", # Line too long - hard to avoid in doctests, and better handled by black. + "E741", # Variable names I, O, l - common in math, and not an issue for most monospace fonts + # Imports at top-level of file - we can't follow this because we intentionally defer imports for various reasons. + # maybe once we can use PEP 810 this can be fixed. + "PLC0415", ] + select = [ + # A starting point for our ruff rules, we may want to ignore some of these later on. "E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e - "F", # pyflakes - https://docs.astral.sh/ruff/rules/#pyflakes-f + "W", # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#pycodestyle-e-w "I", # isort - https://docs.astral.sh/ruff/rules/#isort-i "PL", # pylint - https://docs.astral.sh/ruff/rules/#pylint-pl + + # The remaining rules were added deliberately, and should not be ignored. + # ruff-specific rules -- https://docs.astral.sh/ruff/rules/#ruff-specific-rules-ruf "RUF013", "RUF036", diff --git a/src/sage/coding/linear_code.py b/src/sage/coding/linear_code.py index 6075dbbd785..911a8bc77b9 100644 --- a/src/sage/coding/linear_code.py +++ b/src/sage/coding/linear_code.py @@ -3123,4 +3123,4 @@ def decoding_radius(self): LinearCode._registered_encoders["GeneratorMatrix"] = LinearCodeGeneratorMatrixEncoder LinearCodeSyndromeDecoder._decoder_type = {"hard-decision", "dynamic"} -LinearCodeNearestNeighborDecoder._decoder_type = {"hard-decision", "always-succeed", "complete"} \ No newline at end of file +LinearCodeNearestNeighborDecoder._decoder_type = {"hard-decision", "always-succeed", "complete"} diff --git a/src/sage_docbuild/__main__.py b/src/sage_docbuild/__main__.py index 6459596a28b..21bd219a65a 100644 --- a/src/sage_docbuild/__main__.py +++ b/src/sage_docbuild/__main__.py @@ -459,7 +459,7 @@ def main(): args.source_dir = args.source_dir.absolute() if not args.source_dir.is_dir(): parser.error(f"Source directory {args.source_dir} does not exist.") - + if args.all_documents: if args.all_documents == 'reference': docs = get_all_reference_documents(args.source_dir / 'en') @@ -473,7 +473,7 @@ def main(): # Check that the docs output directory exists if args.output_dir is None: - args.output_dir = Path(os.environ.get('SAGE_DOC', 'src/doc')) + args.output_dir = Path(os.environ.get('SAGE_DOC', 'src/doc')) args.output_dir = args.output_dir.absolute() if not args.output_dir.exists(): try: diff --git a/src/sage_docbuild/builders.py b/src/sage_docbuild/builders.py index cfec1904703..a165fefae72 100644 --- a/src/sage_docbuild/builders.py +++ b/src/sage_docbuild/builders.py @@ -1227,7 +1227,7 @@ def get_all_reference_documents(source: Path) -> list[Path]: n = len(list(directory.iterdir())) documents.append((-n, directory.relative_to(source))) - # Sort largest component (most subdirectory entries) first since + # Sort largest component (most subdirectory entries) first since # they will take the longest to build docs = [doc[1] for doc in sorted(documents)] # Put the bibliography first, because it needs to be built first: diff --git a/src/tox.ini b/src/tox.ini index e83b6a9f389..0a5c0d68bd4 100644 --- a/src/tox.ini +++ b/src/tox.ini @@ -272,50 +272,42 @@ 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": -# -# 10589 PLC0415 [ ] import-outside-top-level -# 3659 PLR2004 [ ] magic-value-comparison -# 2973 I001 [*] unsorted-imports -# 2278 F821 [ ] undefined-name -# 1976 E741 [ ] ambiguous-variable-name -# 1458 PLR0912 [ ] too-many-branches -# 815 PLR0913 [ ] too-many-arguments -# 792 E402 [ ] module-import-not-at-top-of-file -# 744 F405 [ ] undefined-local-with-import-star-usage -# 693 PLR0915 [ ] too-many-statements -# 679 F401 [ ] unused-import -# 494 PLW0211 [ ] bad-staticmethod-argument -# 471 PLW2901 [ ] redefined-loop-name -# 438 PLR0911 [ ] too-many-return-statements -# 383 PLR5501 [*] collapsible-else-if -# 376 E731 [ ] lambda-assignment -# 298 PLR1714 [ ] repeated-equality-comparison -# 226 F403 [ ] undefined-local-with-import-star -# 190 PLW1641 [ ] eq-without-hash -# 94 PLW0603 [ ] global-statement -# 91 PLR0402 [*] manual-from-import -# 61 F841 [ ] unused-variable -# 61 PLW0642 [ ] self-or-cls-assignment -# 56 PLR1704 [ ] redefined-argument-from-local -# 43 PLW0602 [ ] global-variable-not-assigned -# 39 PLC0206 [ ] dict-index-missing-items -# 14 PLW1510 [*] subprocess-run-without-check -# 10 E743 [ ] ambiguous-function-name -# 8 PLW1508 [ ] invalid-envvar-default -# 7 PLR0124 [ ] comparison-with-itself -# 5 E721 [ ] type-comparison -# 5 PLC2401 [ ] non-ascii-name -# 3 PLC3002 [ ] unnecessary-direct-lambda-call -# 3 PLE0302 [ ] unexpected-special-method-signature -# 3 PLW3301 [ ] nested-min-max -# 2 E742 [ ] ambiguous-class-name -# 2 PLR1716 [*] boolean-chained-comparison -# 1 PLC1802 [*] len-test -# 1 PLR1733 [*] unnecessary-dict-index-lookup -# +# 3660 PLR2004 [ ] magic-value-comparison +# 2872 I001 [*] unsorted-imports +# 1456 PLR0912 [ ] too-many-branches +# 817 PLR0913 [ ] too-many-arguments +# 780 E402 [ ] module-import-not-at-top-of-file +# 694 PLR0915 [ ] too-many-statements +# 495 PLW0211 [ ] bad-staticmethod-argument +# 473 PLW2901 [ ] redefined-loop-name +# 439 PLR0911 [ ] too-many-return-statements +# 375 PLR5501 [*] collapsible-else-if +# 372 E731 [ ] lambda-assignment +# 298 PLR1714 [ ] repeated-equality-comparison +# 190 PLW1641 [ ] eq-without-hash +# 162 W292 [*] missing-newline-at-end-of-file +# 94 PLW0603 [ ] global-statement +# 90 PLR0402 [*] manual-from-import +# 61 PLW0642 [ ] self-or-cls-assignment +# 56 PLR1704 [ ] redefined-argument-from-local +# 43 PLW0602 [ ] global-variable-not-assigned +# 39 PLC0206 [ ] dict-index-missing-items +# 16 PLW1510 [*] subprocess-run-without-check +# 10 E743 [ ] ambiguous-function-name +# 8 PLW1508 [ ] invalid-envvar-default +# 7 PLR0124 [ ] comparison-with-itself +# 5 E721 [ ] type-comparison +# 5 PLC2401 [ ] non-ascii-name +# 3 PLC3002 [ ] unnecessary-direct-lambda-call +# 3 PLE0302 [ ] unexpected-special-method-signature +# 3 PLW3301 [ ] nested-min-max +# 2 E742 [ ] ambiguous-class-name +# 2 PLR1716 [*] boolean-chained-comparison +# 1 PLR1711 [*] useless-return +# 1 PLR1733 [*] unnecessary-dict-index-lookup commands = - ruff check --ignore E402,E721,E731,E741,E742,E743,F401,F402,F403,F405,F821,F841,I001,PLC0206,PLC0208,PLC2401,PLC3002,PLE0302,PLR0124,PLR0402,PLR0911,PLR0912,PLR0913,PLR0915,PLR1704,PLR1711,PLR1714,PLR1716,PLR1736,PLR2004,PLR5501,PLW0120,PLW0211,PLW0602,PLW0603,PLW0642,PLW1508,PLW1510,PLW2901,PLW3301 {posargs:{toxinidir}/sage/} - ruff check --preview --select E111,E115,E21,E221,E222,E225,E227,E228,E25,E271,E272,E275,E302,E303,E305,E306,E401,E502,E701,E702,E703,E71,W291,W293,W391,W605,RUF013,RUF036,TC,UP004,UP006,UP008,UP010,UP015,UP022,UP034,UP035 {posargs:{toxinidir}/sage/} + ruff check {posargs:{toxinidir}/sage/} + ruff check --preview {posargs:{toxinidir}/sage/} [flake8] rst-roles = From feea4ab95d1b420d26be6bb237c8ef22214a8cef Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 14:08:32 -0700 Subject: [PATCH 02/18] Fixes --- .github/sync_labels.py | 4 ++-- conftest.py | 3 ++- pyproject.toml | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index 2dbdc671faf..b3938db79ec 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -170,7 +170,7 @@ def is_pull_request(self): def reset_view(self): r""" - Reset cache of ``gh view`` results. + Reset cache of ``gh view`` results. """ self._labels = None self._author = None @@ -942,7 +942,7 @@ def on_review_comment(self): info('Simulate label addition of %s for %s' % (label, self._issue)) self.select_label(status) self.run(Action.labeled, label=status.value) - + def remove_all_labels_of_sel_list(self, sel_list): r""" Remove all labels of given selection list. diff --git a/conftest.py b/conftest.py index 5ab4ef025d4..09ef80641e0 100644 --- a/conftest.py +++ b/conftest.py @@ -11,7 +11,7 @@ import inspect import sys import warnings -from typing import Any, Iterable, Optional, TYPE_CHECKING +from typing import Any, Optional, TYPE_CHECKING import pytest from _pytest.doctest import ( @@ -32,6 +32,7 @@ from sage.doctest.parsing import SageDocTestParser, SageOutputChecker if TYPE_CHECKING: + from collections.abc import Iterable from pathlib import Path diff --git a/pyproject.toml b/pyproject.toml index 0b23479730e..90cc42633c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -240,9 +240,9 @@ exclude = [ # Linting it would just get in the way of adding stricter linter rules # to our actual code. "src/doc/", - # This directory handles bootstrapping the build process, - # and allows Python versions as old as 2.6, so there is no point in linting it. - "build/sage_bootstrap/", + # This directory handles the build process, + # and some parts allow Python versions as old as 2.6, so we don't both linting. + "build/", ] [tool.ruff.lint] From 8025e2e3283b9b9389e8e50b8f422a053c081aa1 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 16:41:00 -0700 Subject: [PATCH 03/18] Tweak workflow --- .github/workflows/lint.yml | 10 +++--- .github/workflows/ruff.toml | 36 +++++++++++++++----- pyproject.toml | 13 ++++++- src/sage/combinat/parallelogram_polyomino.py | 2 +- src/sage/combinat/sloane_functions.py | 2 +- 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e841e430a4a..41de4d3ded0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -31,11 +31,13 @@ jobs: id: deps run: pip install uv - - name: Code style check with ruff-minimal + - name: Code style check with ruff if: (success() || failure()) && steps.deps.outcome == 'success' - run: | - uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml - uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --preview + run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --no-preview + + - name: Code style check with ruff (preview) + if: (success() || failure()) && steps.deps.outcome == 'success' + run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --preview - name: Code style check with relint if: (success() || failure()) && steps.deps.outcome == 'success' diff --git a/.github/workflows/ruff.toml b/.github/workflows/ruff.toml index 800582df293..8b24f02a5ce 100644 --- a/.github/workflows/ruff.toml +++ b/.github/workflows/ruff.toml @@ -3,12 +3,32 @@ extend = "../../pyproject.toml" # Start with the project configuration [lint] # Remove rules that we do not yet follow across the whole codebase. -ignore = [ - "E402", "E721", "E731", "E741", "E742", "E743", - "F401", "F402", "F403", "F405", "F821", "F841", - "I001", - "PLC0206", "PLC0208", "PLC1802", "PLC2401", "PLC3002", "PLC0415", - "PLE0302", - "PLR0124", "PLR0402", "PLR0911", "PLR0912", "PLR0913", "PLR0915", "PLR1704", "PLR1711", "PLR1714", "PLR1716", "PLR1733", "PLR1736", "PLR2004", "PLR5501", - "PLW0120", "PLW0211", "PLW0602", "PLW0603", "PLW0642", "PLW1508", "PLW1510", "PLW1641", "PLW2901", "PLW3301", +# Roughly sorted by how easy these should be to fix. +extend-ignore = [ + "E742", # 2 failures + "PLW3301", # 3 failures, should be easy to autofix + "PLW1510", # 18 failures, should be easy to autofix + "PLR1711", # 4 failures, should be easy to autofix + "PLR1733", # 1 failure, should be easy to autofix + "PLC3002", # 3 failures + "PLE0302", # 3 failures + "E721", # 5 failures + "PLR0124", # 7 failures + "PLW1508", # 8 failures + "PLR0402", # 90 failures, should be easy to autofix + "PLR5501", # 375 failures, should be easy to autofix + "PLC0206", # 39 + "PLW0602", # 43 failures + "PLR1704", # 56 failures + "PLW0642", # 61 failures + "F841", # 61 failures + "PLW0603", # 94 failures + "F403", # 142 failures + "F405", # 163 failures + "E731", # 370 failures + "PLW0211", # 495 failures + "PLW2901", # 474 failures + "F821", # 2271 failures + "F401", # 583 failures + "I001", # 2888 failures ] diff --git a/pyproject.toml b/pyproject.toml index 90cc42633c0..d1a3bdcaedd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -251,10 +251,21 @@ exclude = [ # disagree with them or because following them is not currently practical. ignore = [ "E501", # Line too long - hard to avoid in doctests, and better handled by black. - "E741", # Variable names I, O, l - common in math, and not an issue for most monospace fonts + "E741", "E743", # Variable/function names I, O, l - common in math, and not an issue for most monospace fonts + "PLC2401", # Non-ASCII variables - we deliberately use Greek letters (albeit sparingly) + + # The following rules are good in spirit but require some common sense, better enforced through code review. + "PLR09", # Rules about code complexity - better enforced through code review process. + "PLR1714", # Repeated equality comparison - too picky when only doing a couple comparisons. + "PLR2004", # Comparison to magic values - better enforced through code review process. + "PLW1641", # __eq__ without __hash__ - implementations of equivalence classes may have a well-defined notion of equality without a well-defined canonical representation that can be used to compute a hash + # Imports at top-level of file - we can't follow this because we intentionally defer imports for various reasons. # maybe once we can use PEP 810 this can be fixed. "PLC0415", + # Module level import not at top of file. + # ruff is not aware of our lazy_imports, we may be able to remove this ignore after adopting PEP 810 + "E402", ] select = [ diff --git a/src/sage/combinat/parallelogram_polyomino.py b/src/sage/combinat/parallelogram_polyomino.py index dc72e615107..c0f4bbf9d8b 100644 --- a/src/sage/combinat/parallelogram_polyomino.py +++ b/src/sage/combinat/parallelogram_polyomino.py @@ -2217,7 +2217,7 @@ def cell_is_inside(self, w, h): if h >= len(widths) or h < 0: return 0 - if lower_widths[h] <= w and w < lower_widths[h] + widths[h]: + if lower_widths[h] <= w < lower_widths[h] + widths[h]: return 1 return 0 diff --git a/src/sage/combinat/sloane_functions.py b/src/sage/combinat/sloane_functions.py index f0c61af69cb..20fe45af8c2 100644 --- a/src/sage/combinat/sloane_functions.py +++ b/src/sage/combinat/sloane_functions.py @@ -8087,7 +8087,7 @@ def perm_mh(m, h): A = M(0) for i in range(m): for j in range(n): - if i <= j and j <= i + h: + if i <= j <= i + h: A[i, j] = 1 return A.permanent() From 2f00d983c68a0775d3f0d6ebc23009aec6442955 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 16:45:19 -0700 Subject: [PATCH 04/18] Skip preview check if no preview failed --- .github/workflows/lint.yml | 2 +- pyproject.toml | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 41de4d3ded0..f1e2e30ce66 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -36,7 +36,7 @@ jobs: run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --no-preview - name: Code style check with ruff (preview) - if: (success() || failure()) && steps.deps.outcome == 'success' + if: success() run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --preview - name: Code style check with relint diff --git a/pyproject.toml b/pyproject.toml index d1a3bdcaedd..30d611dd16a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -251,14 +251,14 @@ exclude = [ # disagree with them or because following them is not currently practical. ignore = [ "E501", # Line too long - hard to avoid in doctests, and better handled by black. - "E741", "E743", # Variable/function names I, O, l - common in math, and not an issue for most monospace fonts - "PLC2401", # Non-ASCII variables - we deliberately use Greek letters (albeit sparingly) - - # The following rules are good in spirit but require some common sense, better enforced through code review. - "PLR09", # Rules about code complexity - better enforced through code review process. - "PLR1714", # Repeated equality comparison - too picky when only doing a couple comparisons. - "PLR2004", # Comparison to magic values - better enforced through code review process. - "PLW1641", # __eq__ without __hash__ - implementations of equivalence classes may have a well-defined notion of equality without a well-defined canonical representation that can be used to compute a hash +# "E741", "E743", # Variable/function names I, O, l - common in math, and not an issue for most monospace fonts +# "PLC2401", # Non-ASCII variables - we deliberately use Greek letters (albeit sparingly) +# +# # The following rules are good in spirit but require some common sense, better enforced through code review. +# "PLR09", # Rules about code complexity - better enforced through code review process. +# "PLR1714", # Repeated equality comparison - too picky when only doing a couple comparisons. +# "PLR2004", # Comparison to magic values - better enforced through code review process. +# "PLW1641", # __eq__ without __hash__ - implementations of equivalence classes may have a well-defined notion of equality without a well-defined canonical representation that can be used to compute a hash # Imports at top-level of file - we can't follow this because we intentionally defer imports for various reasons. # maybe once we can use PEP 810 this can be fixed. From e44886c681cc403a70f1c75539178194775b000f Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 16:51:15 -0700 Subject: [PATCH 05/18] Cleanup --- .github/workflows/lint.yml | 9 +++++---- pyproject.toml | 16 ++++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f1e2e30ce66..bbb0118fb64 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -32,19 +32,20 @@ jobs: run: pip install uv - name: Code style check with ruff - if: (success() || failure()) && steps.deps.outcome == 'success' + if: ${{ !cancelled() && steps.deps.outcome == 'success' }} + id: ruff_no_preview run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --no-preview - name: Code style check with ruff (preview) - if: success() + if: ${{ !cancelled() && steps.ruff_no_preview.outcome == 'success' }} run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --preview - name: Code style check with relint - if: (success() || failure()) && steps.deps.outcome == 'success' + if: ${{ !cancelled() && steps.deps.outcome == 'success' }} run: uv run --frozen --only-group lint -- relint -c src/.relint.yml -- src/sage/ - name: Validate docstring markup as RST - if: (success() || failure()) && steps.deps.outcome == 'success' + if: ${{ !cancelled() && steps.deps.outcome == 'success' }} run: uv run --frozen --only-group lint -- flake8 --select=RST src/sage/ --config src/tox.ini - name: TOML format check with taplo diff --git a/pyproject.toml b/pyproject.toml index 30d611dd16a..d1a3bdcaedd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -251,14 +251,14 @@ exclude = [ # disagree with them or because following them is not currently practical. ignore = [ "E501", # Line too long - hard to avoid in doctests, and better handled by black. -# "E741", "E743", # Variable/function names I, O, l - common in math, and not an issue for most monospace fonts -# "PLC2401", # Non-ASCII variables - we deliberately use Greek letters (albeit sparingly) -# -# # The following rules are good in spirit but require some common sense, better enforced through code review. -# "PLR09", # Rules about code complexity - better enforced through code review process. -# "PLR1714", # Repeated equality comparison - too picky when only doing a couple comparisons. -# "PLR2004", # Comparison to magic values - better enforced through code review process. -# "PLW1641", # __eq__ without __hash__ - implementations of equivalence classes may have a well-defined notion of equality without a well-defined canonical representation that can be used to compute a hash + "E741", "E743", # Variable/function names I, O, l - common in math, and not an issue for most monospace fonts + "PLC2401", # Non-ASCII variables - we deliberately use Greek letters (albeit sparingly) + + # The following rules are good in spirit but require some common sense, better enforced through code review. + "PLR09", # Rules about code complexity - better enforced through code review process. + "PLR1714", # Repeated equality comparison - too picky when only doing a couple comparisons. + "PLR2004", # Comparison to magic values - better enforced through code review process. + "PLW1641", # __eq__ without __hash__ - implementations of equivalence classes may have a well-defined notion of equality without a well-defined canonical representation that can be used to compute a hash # Imports at top-level of file - we can't follow this because we intentionally defer imports for various reasons. # maybe once we can use PEP 810 this can be fixed. From 5e4de08f86cd0d9f53453151ae40d8750be98ab9 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 16:58:45 -0700 Subject: [PATCH 06/18] Add more rules to CI ignore --- .github/workflows/ruff.toml | 27 ++++++++++++++++++++++++++- conftest.py | 2 +- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ruff.toml b/.github/workflows/ruff.toml index 8b24f02a5ce..4e9b83a5eb5 100644 --- a/.github/workflows/ruff.toml +++ b/.github/workflows/ruff.toml @@ -31,4 +31,29 @@ extend-ignore = [ "F821", # 2271 failures "F401", # 583 failures "I001", # 2888 failures -] + + # The remaining failing rules were in ruff preview mode only when added to this file + "E231", # missing-whitespace + "E226", # missing-whitespace-around-arithmetic-operator + "PLW3201", # bad-dunder-method-name + "PLR6301", # no-self-use + "E241", # multiple-spaces-after-comma + "E203", # whitespace-before-punctuation + "E265", # no-space-after-block-comment + "E201", # whitespace-after-open-bracket + "E261", # too-few-spaces-before-inline-comment + "PLR6104", # non-augmented-assignment + "E202", # whitespace-before-close-bracket + "PLR6201", # literal-membership + "E266", # multiple-leading-hashes-for-block-comment + "PLR1702", # too-many-nested-blocks + "E262", # no-space-after-inline-comment + "PLW1514", # unspecified-encoding + "PLC1901", # compare-to-empty-string + "PLC2801", # unnecessary-dunder-call + "E116", # unexpected-indentation-comment + "E114", # indentation-with-invalid-multiple-comment + "PLW0108", # unnecessary-lambda + "PLC2701", # import-private-name + "PLE4703", # modified-iterating-set + ] diff --git a/conftest.py b/conftest.py index 09ef80641e0..cb12785943c 100644 --- a/conftest.py +++ b/conftest.py @@ -271,7 +271,7 @@ def pytest_collect_file( def pytest_ignore_collect( collection_path: Path, path: str, config: pytest.Config -) -> None | bool: +) -> bool | None: """ This hook is called when collecting test files, and can be used to prevent considering this path for collection by returning ``True``. From 572ebf92f0b36b91ea4429e3998790bf919eb895 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 16:59:39 -0700 Subject: [PATCH 07/18] Fix PLR1711 --- .github/sync_labels.py | 3 --- .github/workflows/ruff.toml | 1 - src/sage/groups/cubic_braid.py | 1 - 3 files changed, 5 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index b3938db79ec..89421fc5a0b 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -796,7 +796,6 @@ def reject_label_addition(self, item): if item is Status.positive_review: self.add_warning('Label *%s* cannot be added by the author of the PR.' % item.value) self.remove_label(item.value) - return def warning_about_label_addition(self, item): r""" @@ -808,7 +807,6 @@ def warning_about_label_addition(self, item): self.add_warning('Label *%s* may be incorrect, since there are unresolved reviews.' % item.value) else: self.add_warning('Label *%s* does not match the state of GitHub\'s review system.' % item.value) - return def hint_about_label_removal(self, item): r""" @@ -819,7 +817,6 @@ def hint_about_label_removal(self, item): else: sel_list = 'priority' self.add_hint('You don\'t need to remove %s labels any more. You\'d better just add the label which replaces it.' % sel_list) - return # ------------------------------------------------------------------------- # methods to act on events diff --git a/.github/workflows/ruff.toml b/.github/workflows/ruff.toml index 4e9b83a5eb5..d4fe0bc557a 100644 --- a/.github/workflows/ruff.toml +++ b/.github/workflows/ruff.toml @@ -8,7 +8,6 @@ extend-ignore = [ "E742", # 2 failures "PLW3301", # 3 failures, should be easy to autofix "PLW1510", # 18 failures, should be easy to autofix - "PLR1711", # 4 failures, should be easy to autofix "PLR1733", # 1 failure, should be easy to autofix "PLC3002", # 3 failures "PLE0302", # 3 failures diff --git a/src/sage/groups/cubic_braid.py b/src/sage/groups/cubic_braid.py index 899c9c9bc1b..0e7c5dc78e5 100644 --- a/src/sage/groups/cubic_braid.py +++ b/src/sage/groups/cubic_braid.py @@ -990,7 +990,6 @@ def _test_matrix_group(self, **options): matrix_grpF7 = self.as_matrix_group(domain=GF(7)) self._internal_test_attached_group(matrix_grpF7, tester) - return def _test_reflection_group(self, **options): r""" From 0024305e07deb42be890df25aa9f05eb96a6fdfc Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 17:00:49 -0700 Subject: [PATCH 08/18] Fix ruff PLR1733 --- .github/workflows/ruff.toml | 1 - .../lie_conformal_algebra_with_structure_coefs.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ruff.toml b/.github/workflows/ruff.toml index d4fe0bc557a..38e592c5fba 100644 --- a/.github/workflows/ruff.toml +++ b/.github/workflows/ruff.toml @@ -8,7 +8,6 @@ extend-ignore = [ "E742", # 2 failures "PLW3301", # 3 failures, should be easy to autofix "PLW1510", # 18 failures, should be easy to autofix - "PLR1733", # 1 failure, should be easy to autofix "PLC3002", # 3 failures "PLE0302", # 3 failures "E721", # 5 failures diff --git a/src/sage/algebras/lie_conformal_algebras/lie_conformal_algebra_with_structure_coefs.py b/src/sage/algebras/lie_conformal_algebras/lie_conformal_algebra_with_structure_coefs.py index 2e7f31cee27..5bfb795a829 100644 --- a/src/sage/algebras/lie_conformal_algebras/lie_conformal_algebra_with_structure_coefs.py +++ b/src/sage/algebras/lie_conformal_algebras/lie_conformal_algebra_with_structure_coefs.py @@ -227,7 +227,7 @@ def __init__(self, R, s_coeff, index_set=None, central_elements=None, # convert to index_set as keys s_coeff = {(d[k[0]], d[k[1]]): {a: {(d[x[1]], x[2]): sck[a][x] for x in sck[a]} - for a in s_coeff[k]} + for a in sck} for k, sck in s_coeff.items()} except KeyError: From 2202a87bd5dc1f8eb9b3ec2a26d5636eb2dd38b4 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 17:09:39 -0700 Subject: [PATCH 09/18] Ruff tweaks --- .github/workflows/ruff.toml | 10 +++++----- src/tox.ini | 13 ------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ruff.toml b/.github/workflows/ruff.toml index 38e592c5fba..db2736af4d2 100644 --- a/.github/workflows/ruff.toml +++ b/.github/workflows/ruff.toml @@ -27,12 +27,12 @@ extend-ignore = [ "PLW0211", # 495 failures "PLW2901", # 474 failures "F821", # 2271 failures - "F401", # 583 failures - "I001", # 2888 failures + "E231", # 49042 failures, could be autofixed but would change too many files for one PR + "E226", # 29323 failures, could be autofixed but would change too many files for one PR + "F401", # 583 failures + "I001", # 2888 failures - # The remaining failing rules were in ruff preview mode only when added to this file - "E231", # missing-whitespace - "E226", # missing-whitespace-around-arithmetic-operator + # The remaining failing rules were in ruff preview mode only when added to this file. "PLW3201", # bad-dunder-method-name "PLR6301", # no-self-use "E241", # multiple-spaces-after-comma diff --git a/src/tox.ini b/src/tox.ini index 0a5c0d68bd4..f807d72bb1d 100644 --- a/src/tox.ini +++ b/src/tox.ini @@ -272,19 +272,11 @@ 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": -# 3660 PLR2004 [ ] magic-value-comparison # 2872 I001 [*] unsorted-imports -# 1456 PLR0912 [ ] too-many-branches -# 817 PLR0913 [ ] too-many-arguments -# 780 E402 [ ] module-import-not-at-top-of-file -# 694 PLR0915 [ ] too-many-statements # 495 PLW0211 [ ] bad-staticmethod-argument # 473 PLW2901 [ ] redefined-loop-name -# 439 PLR0911 [ ] too-many-return-statements # 375 PLR5501 [*] collapsible-else-if # 372 E731 [ ] lambda-assignment -# 298 PLR1714 [ ] repeated-equality-comparison -# 190 PLW1641 [ ] eq-without-hash # 162 W292 [*] missing-newline-at-end-of-file # 94 PLW0603 [ ] global-statement # 90 PLR0402 [*] manual-from-import @@ -293,18 +285,13 @@ passenv = RUFF_OUTPUT_FORMAT # 43 PLW0602 [ ] global-variable-not-assigned # 39 PLC0206 [ ] dict-index-missing-items # 16 PLW1510 [*] subprocess-run-without-check -# 10 E743 [ ] ambiguous-function-name # 8 PLW1508 [ ] invalid-envvar-default # 7 PLR0124 [ ] comparison-with-itself # 5 E721 [ ] type-comparison -# 5 PLC2401 [ ] non-ascii-name # 3 PLC3002 [ ] unnecessary-direct-lambda-call # 3 PLE0302 [ ] unexpected-special-method-signature # 3 PLW3301 [ ] nested-min-max # 2 E742 [ ] ambiguous-class-name -# 2 PLR1716 [*] boolean-chained-comparison -# 1 PLR1711 [*] useless-return -# 1 PLR1733 [*] unnecessary-dict-index-lookup commands = ruff check {posargs:{toxinidir}/sage/} ruff check --preview {posargs:{toxinidir}/sage/} From 92cf8f1e735b513a47f8257b7fd4d7a50261ca6d Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 17:18:45 -0700 Subject: [PATCH 10/18] Fix a few more preview rules --- .github/sync_labels.py | 5 ++-- .github/workflows/ruff.toml | 46 ++++++++++++++++++---------------- pkgs/sagemath-objects/setup.py | 2 +- tools/update-conda.py | 1 + tools/update-meson.py | 2 +- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index 89421fc5a0b..904460fc554 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -187,7 +187,7 @@ def rest_api(self, path_args, method=None, query=''): """ meth = '-X GET' if method: - meth='-X %s' % method + meth = '-X %s' % method cmd = 'gh api %s -H \"Accept: application/vnd.github+json\" %s %s' % (meth, path_args, query) debug('Execute command: %s' % cmd) if method: @@ -248,6 +248,7 @@ def bot_login(self): outtxt = str(capt.stdout) debug('auth status err: %s' % errtxt) debug('auth status out: %s' % outtxt) + def read_login(txt, position_mark): for t in txt: for p in position_mark: @@ -413,7 +414,7 @@ def get_commits(self): date_commits = list(self._commits) if Status.positive_review.value not in self.get_labels(): for com in self._commits: - message = com['messageHeadline'] + message = com['messageHeadline'] if message.startswith('Merge') and 'develop' in message: debug('Ignore merge commit %s for commit_date' % com['oid']) date_commits.remove(com) diff --git a/.github/workflows/ruff.toml b/.github/workflows/ruff.toml index db2736af4d2..f58cb0f3d38 100644 --- a/.github/workflows/ruff.toml +++ b/.github/workflows/ruff.toml @@ -33,25 +33,27 @@ extend-ignore = [ "I001", # 2888 failures # The remaining failing rules were in ruff preview mode only when added to this file. - "PLW3201", # bad-dunder-method-name - "PLR6301", # no-self-use - "E241", # multiple-spaces-after-comma - "E203", # whitespace-before-punctuation - "E265", # no-space-after-block-comment - "E201", # whitespace-after-open-bracket - "E261", # too-few-spaces-before-inline-comment - "PLR6104", # non-augmented-assignment - "E202", # whitespace-before-close-bracket - "PLR6201", # literal-membership - "E266", # multiple-leading-hashes-for-block-comment - "PLR1702", # too-many-nested-blocks - "E262", # no-space-after-inline-comment - "PLW1514", # unspecified-encoding - "PLC1901", # compare-to-empty-string - "PLC2801", # unnecessary-dunder-call - "E116", # unexpected-indentation-comment - "E114", # indentation-with-invalid-multiple-comment - "PLW0108", # unnecessary-lambda - "PLC2701", # import-private-name - "PLE4703", # modified-iterating-set - ] + "PLW3201", + "PLR6301", + "E241", + "E203", + "E265", + "E201", + "E261", + "PLR6104", + "E202", + "PLR6201", + "E266", + "PLR1702", + "E262", + "PLW1514", + "PLC1901", + "PLC2801", + "E116", + "E114", + "PLC2701", + "E251", + "PLW0108", + "E302", + "PLE4703", +] diff --git a/pkgs/sagemath-objects/setup.py b/pkgs/sagemath-objects/setup.py index ad114fa0de1..7619a4b2b37 100644 --- a/pkgs/sagemath-objects/setup.py +++ b/pkgs/sagemath-objects/setup.py @@ -40,6 +40,6 @@ setup( cmdclass = cmdclass, packages = python_packages, - py_modules = python_modules, + py_modules = python_modules, ext_modules = cython_modules, ) diff --git a/tools/update-conda.py b/tools/update-conda.py index 3dd18445502..045a068437c 100644 --- a/tools/update-conda.py +++ b/tools/update-conda.py @@ -324,4 +324,5 @@ def get_optional_dependencies(pyproject: dict) -> list[str]: # print(f"Optional dependencies: {optional_dependencies}") # Uncommented for debugging return optional_dependencies + update_conda(options.sourcedir, options.systems) diff --git a/tools/update-meson.py b/tools/update-meson.py index 72c132c2f41..4bbf28340eb 100755 --- a/tools/update-meson.py +++ b/tools/update-meson.py @@ -213,7 +213,7 @@ def update_doc_sources(self: Rewriter, visitor: AstPython): if not dirs and not files: folder.rmdir() - for folder, dirs, files in doc_folder.walk(): + for folder, dirs, files in doc_folder.walk(): if folder.name in ignored_folders or folder == doc_folder: continue files_to_add = {} From a8e04e8fc3adec3b55d82648da3a869089d14cf7 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 17:21:29 -0700 Subject: [PATCH 11/18] Print ruff version --- .github/workflows/lint.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index bbb0118fb64..e9432342524 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -31,6 +31,10 @@ jobs: id: deps run: pip install uv + - name: Print ruff version + if: ${{ !cancelled() && steps.deps.outcome == 'success' }} + run: uv run --frozen --only-group lint -- ruff --version + - name: Code style check with ruff if: ${{ !cancelled() && steps.deps.outcome == 'success' }} id: ruff_no_preview From 9b2924c377f58a0bfa2c4dae2da81d5267993559 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 17:37:48 -0700 Subject: [PATCH 12/18] List all rules manually --- pyproject.toml | 120 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 115 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d1a3bdcaedd..bbf848bd88b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -270,17 +270,14 @@ ignore = [ select = [ # A starting point for our ruff rules, we may want to ignore some of these later on. - "E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e - "W", # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#pycodestyle-e-w "I", # isort - https://docs.astral.sh/ruff/rules/#isort-i - "PL", # pylint - https://docs.astral.sh/ruff/rules/#pylint-pl - - # The remaining rules were added deliberately, and should not be ignored. # ruff-specific rules -- https://docs.astral.sh/ruff/rules/#ruff-specific-rules-ruf "RUF013", "RUF036", + "TC", # flake8-type-checking - https://docs.astral.sh/ruff/rules/#flake8-type-checking-tc + # pyupgrade -- https://docs.astral.sh/ruff/rules/#pyupgrade-up "UP004", "UP006", @@ -290,6 +287,119 @@ select = [ "UP022", "UP034", "UP035", + + "W", # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#pycodestyle-e-w + + # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e + # Ideally we would enable (almost) all of the E rules, but too + # many of these rules are in preview to trust that something won't change + # in ruff itself and cause us headaches. + "E101", + "E401", + "E701", + "E702", + "E703", + "E711", + "E712", + "E713", + "E714", + "E721", + "E722", + "E731", + "E742", + "E902", + + # pylint - https://docs.astral.sh/ruff/rules/#pylint-pl + # Like with the E rules, this isn't stable enough yet to enable all of PL + + # pylint convention - https://docs.astral.sh/ruff/rules/#convention-plc + "PLC0105", + "PLC0131", + "PLC0132", + "PLC0205", + "PLC0206", + "PLC0208", + "PLC0414", + "PLC1802", + "PLC2403", + "PLC3002", + + # pylint errors - https://docs.astral.sh/ruff/rules/#error-ple + "PLE0100", + "PLE0101", + "PLE0115", + "PLE0116", + "PLE0117", + "PLE0118", + "PLE0237", + "PLE0241", + "PLE0302", + "PLE0303", + "PLE0305", + "PLE0307", + "PLE0308", + "PLE0309", + "PLE0604", + "PLE0605", + "PLE0643", + "PLE0704", + "PLE1132", + "PLE1142", + "PLE1205", + "PLE1206", + "PLE1300", + "PLE1307", + "PLE1310", + "PLE1507", + "PLE1519", + "PLE1520", + "PLE1700", + "PLE2502", + "PLE2510", + "PLE2512", + "PLE2513", + "PLE2514", + "PLE2515", + + # pylint refactor - https://docs.astral.sh/ruff/rules/#refactor-plr + "PLR0124", + "PLR0133", + "PLR0206", + "PLR0402", + "PLR1704", + "PLR1711", + "PLR1716", + "PLR1722", + "PLR1730", + "PLR1733", + "PLR1736", + "PLR2044", + "PLR5501", + + # pyling warning - https://docs.astral.sh/ruff/rules/#warning-plw + "PLW0120", + "PLW0127", + "PLW0128", + "PLW0129", + "PLW0131", + "PLW0133", + "PLW0177", + "PLW0211", + "PLW0245", + "PLW0406", + "PLW0602", + "PLW0603", + "PLW0604", + "PLW0642", + "PLW0711", + "PLW1501", + "PLW1507", + "PLW1508", + "PLW1509", + "PLW1510", + "PLW2101", + "PLW2901", + "PLW3301", ] [tool.ruff.lint.per-file-ignores] From 2519d31871e5634ad3c301de3a832557bc795747 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 3 Dec 2025 17:39:31 -0700 Subject: [PATCH 13/18] Remove debugging rule --- .github/workflows/lint.yml | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e9432342524..ae83349d4bd 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -31,18 +31,11 @@ jobs: id: deps run: pip install uv - - name: Print ruff version - if: ${{ !cancelled() && steps.deps.outcome == 'success' }} - run: uv run --frozen --only-group lint -- ruff --version - - name: Code style check with ruff if: ${{ !cancelled() && steps.deps.outcome == 'success' }} - id: ruff_no_preview - run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --no-preview - - - name: Code style check with ruff (preview) - if: ${{ !cancelled() && steps.ruff_no_preview.outcome == 'success' }} - run: uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --preview + run: | + uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --no-preview + uv run --frozen --only-group lint -- ruff check --output-format github --config .github/workflows/ruff.toml --preview - name: Code style check with relint if: ${{ !cancelled() && steps.deps.outcome == 'success' }} From 3b467a73f8d6c185c0a876f925f922762129a82a Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Thu, 4 Dec 2025 13:31:04 -0700 Subject: [PATCH 14/18] TOML formatting --- pyproject.toml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index bbf848bd88b..10f8ccdcf76 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -251,7 +251,11 @@ exclude = [ # disagree with them or because following them is not currently practical. ignore = [ "E501", # Line too long - hard to avoid in doctests, and better handled by black. - "E741", "E743", # Variable/function names I, O, l - common in math, and not an issue for most monospace fonts + + # Variable/function names I, O, l - common in math, and not an issue for most monospace fonts + "E741", + "E743", + "PLC2401", # Non-ASCII variables - we deliberately use Greek letters (albeit sparingly) # The following rules are good in spirit but require some common sense, better enforced through code review. @@ -263,6 +267,7 @@ ignore = [ # Imports at top-level of file - we can't follow this because we intentionally defer imports for various reasons. # maybe once we can use PEP 810 this can be fixed. "PLC0415", + # Module level import not at top of file. # ruff is not aware of our lazy_imports, we may be able to remove this ignore after adopting PEP 810 "E402", From a58fcf974b3a92c9a610c364b42836a0e9fbdcfa Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Thu, 4 Dec 2025 13:35:48 -0700 Subject: [PATCH 15/18] Space --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 10f8ccdcf76..c50416c81ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -270,7 +270,7 @@ ignore = [ # Module level import not at top of file. # ruff is not aware of our lazy_imports, we may be able to remove this ignore after adopting PEP 810 - "E402", + "E402", ] select = [ From 9858891741f9c2b6538e91fa8f0363c45f2ee364 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Sat, 6 Dec 2025 15:41:07 -0700 Subject: [PATCH 16/18] Remove ruff from tox, update developer tools docs --- src/doc/en/developer/tools.rst | 43 +++++++++++++++++++++------------- src/tox.ini | 40 +------------------------------ tox.ini | 20 ---------------- 3 files changed, 28 insertions(+), 75 deletions(-) diff --git a/src/doc/en/developer/tools.rst b/src/doc/en/developer/tools.rst index 59b163e8b74..705b7b07bdf 100644 --- a/src/doc/en/developer/tools.rst +++ b/src/doc/en/developer/tools.rst @@ -64,7 +64,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,ruff-minimal + doctest,coverage,startuptime,pycodestyle-minimal,relint,codespell,rst doctest -- run the Sage doctester (same as "sage -t") coverage -- give information about doctest coverage of files @@ -75,13 +75,11 @@ 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 @@ -310,20 +308,33 @@ 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. +Sage we have two configuration files for ruff. `pyproject.toml` in the root of the +repository defines all rules we wish to follow. `.github/workflows/ruff.toml` takes +the configuration in `pyproject.toml` and disables all rules that we do not already +follow throughout the repository. Our lint GitHub Action workflow requires +``ruff check --config .github/workflows/ruff.toml --preview`` to pass. To speed up the +code review process, developers should verify that this passes locally before submitting a PR. +To make sure you are running the same version of `ruff` locally as GitHub Actions, use the command +``uv run --frozen --only-group lint -- ruff check --config .github/workflows/ruff.toml --preview``. + +Developers are encouraged to locally run ``ruff check [path to changed files]`` +to run the stricter configuration defined in `pyproject.toml` and fix any linter +failures on their new code. This will help to avoid follow-up formatting PRs as +Sage moves toward full PEP 8 compliance. Developers may also choose to fix existing +linter failures on files that they modify, but use common sense when deciding whether +or not to do so. A small bug fix PR should not include a large number of +code-style changes as this makes it harder for reviewers to evaluate the important changes. + +When working on PRs to improve our alignment with our linter rules, the ``--statistics`` +option can be passed to ``ruff`` to print out a list of all rules that are enabled in +`pyproject.toml` but are not currently followed throughout the repository and how many +times each rule is violated. This is useful for finding low-hanging fruit for formatting PRs. +Developers can also use ``--select [RULE CODES]`` to override the list of rules enabled in +`pyproject.toml` when testing additional rules to add to `pyproject.toml`, or +``--select-extend [RULE CODES]`` to add new rules to the existing confirmation. +See the `Ruff documentation `_ to see all features and rules +available. -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 f807d72bb1d..625479ec068 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, ruff-minimal +envlist = doctest, coverage, startuptime, pycodestyle-minimal, relint, codespell, rst # When adding environments above, also update the delegations in SAGE_ROOT/tox.ini skipsdist = true @@ -258,44 +258,6 @@ 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 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": -# 2872 I001 [*] unsorted-imports -# 495 PLW0211 [ ] bad-staticmethod-argument -# 473 PLW2901 [ ] redefined-loop-name -# 375 PLR5501 [*] collapsible-else-if -# 372 E731 [ ] lambda-assignment -# 162 W292 [*] missing-newline-at-end-of-file -# 94 PLW0603 [ ] global-statement -# 90 PLR0402 [*] manual-from-import -# 61 PLW0642 [ ] self-or-cls-assignment -# 56 PLR1704 [ ] redefined-argument-from-local -# 43 PLW0602 [ ] global-variable-not-assigned -# 39 PLC0206 [ ] dict-index-missing-items -# 16 PLW1510 [*] subprocess-run-without-check -# 8 PLW1508 [ ] invalid-envvar-default -# 7 PLR0124 [ ] comparison-with-itself -# 5 E721 [ ] type-comparison -# 3 PLC3002 [ ] unnecessary-direct-lambda-call -# 3 PLE0302 [ ] unexpected-special-method-signature -# 3 PLW3301 [ ] nested-min-max -# 2 E742 [ ] ambiguous-class-name -commands = - ruff check {posargs:{toxinidir}/sage/} - ruff check --preview {posargs:{toxinidir}/sage/} - [flake8] rst-roles = # Sphinx diff --git a/tox.ini b/tox.ini index 385d42e1260..fe964e1c2a0 100644 --- a/tox.ini +++ b/tox.ini @@ -1098,23 +1098,3 @@ 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/} From e1f8a9b94ebafda451023b4729063a7dcb9f8979 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Sun, 7 Dec 2025 17:28:36 -0700 Subject: [PATCH 17/18] Formatting --- src/doc/en/developer/tools.rst | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/doc/en/developer/tools.rst b/src/doc/en/developer/tools.rst index 705b7b07bdf..cedc9067f04 100644 --- a/src/doc/en/developer/tools.rst +++ b/src/doc/en/developer/tools.rst @@ -14,7 +14,7 @@ uv `uv `_ is a versatile tool for managing and synchronizing project dependencies. -The lockfile `uv.lock` in the root captures the exact package versions for +The lockfile ``uv.lock`` in the root captures the exact package versions for all systems and ensures consistent, reproducible installations. It is automatically updated during ``uv`` operations like ``uv add`` and ``uv run``, or explicitly with ``uv lock``. @@ -308,17 +308,17 @@ 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 we have two configuration files for ruff. `pyproject.toml` in the root of the -repository defines all rules we wish to follow. `.github/workflows/ruff.toml` takes -the configuration in `pyproject.toml` and disables all rules that we do not already +Sage we have two configuration files for ruff. ``pyproject.toml`` in the root of the +repository defines all rules we wish to follow. ``.github/workflows/ruff.toml`` takes +the configuration in ``pyproject.toml`` and disables all rules that we do not already follow throughout the repository. Our lint GitHub Action workflow requires ``ruff check --config .github/workflows/ruff.toml --preview`` to pass. To speed up the code review process, developers should verify that this passes locally before submitting a PR. -To make sure you are running the same version of `ruff` locally as GitHub Actions, use the command +To make sure you are running the same version of ``ruff`` locally as GitHub Actions, use the command ``uv run --frozen --only-group lint -- ruff check --config .github/workflows/ruff.toml --preview``. Developers are encouraged to locally run ``ruff check [path to changed files]`` -to run the stricter configuration defined in `pyproject.toml` and fix any linter +to run the stricter configuration defined in ``pyproject.toml`` and fix any linter failures on their new code. This will help to avoid follow-up formatting PRs as Sage moves toward full PEP 8 compliance. Developers may also choose to fix existing linter failures on files that they modify, but use common sense when deciding whether @@ -330,7 +330,7 @@ option can be passed to ``ruff`` to print out a list of all rules that are enabl `pyproject.toml` but are not currently followed throughout the repository and how many times each rule is violated. This is useful for finding low-hanging fruit for formatting PRs. Developers can also use ``--select [RULE CODES]`` to override the list of rules enabled in -`pyproject.toml` when testing additional rules to add to `pyproject.toml`, or +`pyproject.toml` when testing additional rules to add to ``pyproject.toml``, or ``--select-extend [RULE CODES]`` to add new rules to the existing confirmation. See the `Ruff documentation `_ to see all features and rules available. From fe61df10c31c442ad1ab349c838ceaf05b021797 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Sun, 7 Dec 2025 18:36:56 -0700 Subject: [PATCH 18/18] Formatting --- src/doc/en/developer/tools.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/doc/en/developer/tools.rst b/src/doc/en/developer/tools.rst index cedc9067f04..fbd681a40bc 100644 --- a/src/doc/en/developer/tools.rst +++ b/src/doc/en/developer/tools.rst @@ -327,10 +327,10 @@ code-style changes as this makes it harder for reviewers to evaluate the importa When working on PRs to improve our alignment with our linter rules, the ``--statistics`` option can be passed to ``ruff`` to print out a list of all rules that are enabled in -`pyproject.toml` but are not currently followed throughout the repository and how many +``pyproject.toml`` but are not currently followed throughout the repository and how many times each rule is violated. This is useful for finding low-hanging fruit for formatting PRs. Developers can also use ``--select [RULE CODES]`` to override the list of rules enabled in -`pyproject.toml` when testing additional rules to add to ``pyproject.toml``, or +``pyproject.toml`` when testing additional rules to add to ``pyproject.toml``, or ``--select-extend [RULE CODES]`` to add new rules to the existing confirmation. See the `Ruff documentation `_ to see all features and rules available.