diff --git a/.github/sync_labels.py b/.github/sync_labels.py index 2dbdc671faf..904460fc554 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 @@ -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) @@ -796,7 +797,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 +808,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 +818,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 @@ -942,7 +940,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/.github/workflows/lint.yml b/.github/workflows/lint.yml index 94e87ccafdf..ae83349d4bd 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -31,18 +31,18 @@ jobs: id: deps run: pip install uv - - name: Code style check with ruff-minimal - if: (success() || failure()) && steps.deps.outcome == 'success' + - name: Code style check with ruff + if: ${{ !cancelled() && 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 --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: (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/.github/workflows/ruff.toml b/.github/workflows/ruff.toml new file mode 100644 index 00000000000..f58cb0f3d38 --- /dev/null +++ b/.github/workflows/ruff.toml @@ -0,0 +1,59 @@ +# 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. +# 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 + "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 + "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. + "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/conftest.py b/conftest.py index 5ab4ef025d4..cb12785943c 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 @@ -270,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``. 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/pyproject.toml b/pyproject.toml index 3c098ee1494..46d437f1523 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -236,24 +236,54 @@ 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 the build process, + # and some parts allow Python versions as old as 2.6, so we don't both linting. + "build/", +] [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. + + # 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. + "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 = [ - "E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e - "F", # pyflakes - https://docs.astral.sh/ruff/rules/#pyflakes-f + # A starting point for our ruff rules, we may want to ignore some of these later on. "I", # isort - https://docs.astral.sh/ruff/rules/#isort-i - "PL", # pylint - https://docs.astral.sh/ruff/rules/#pylint-pl + # 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", @@ -263,6 +293,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] 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: 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/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() 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""" 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..f807d72bb1d 100644 --- a/src/tox.ini +++ b/src/tox.ini @@ -272,50 +272,29 @@ 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 -# +# 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 --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 = 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 = {}