Skip to content

Commit

Permalink
sagemathgh-37452: Add config for ruff (sagemath#36503, unbundled)
Browse files Browse the repository at this point in the history
    
<!-- ^^^^^
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 -->
Author: @mkoeppe, based on @tobiasdiez's config in sagemath#36503.

Adding a configuration for the ruff linter as proposed in sagemath#36503 is
timely and uncontroversial.

However, sagemath#36503 bundled this gratuitously with the controversial design
of creating a `pyproject.toml` file in SAGE_ROOT.

`pyproject.toml` -- by definition in PEP 518, PEP 621 -- marks a
directory as a Python project, i.e., the source directory of a Python
distribution package
(sagemath#36503 (comment)).
Generalizing the use of `pyproject.toml` to "[non-package
projects](https://peps.python.org/pep-0735/#motivation)" is still
subject to discussion in the Python community in [PEP
735](https://peps.python.org/pep-0735/) (Nov 2023).

**The scope of PRs should be chosen to minimize friction, not to
maximize friction.**
sagemath#36726 (comment)
Here we remove the artificial friction from the gratuitous bundling, by
configuring ruff in `ruff.toml` instead. Configuration in ruff.toml and
pyproject.toml has equal validity / standing per [ruff
documentation](https://docs.astral.sh/ruff/configuration/#config-file-
discovery). And this is the location of most of our other linter
configuration files.

Reference on previous common denominator PRs:
sagemath#36666 (comment),
sagemath#36666 (comment),
sagemath#36572 (comment),
sagemath#36960 (comment),
sagemath#36960 (comment), ...

<!-- 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: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37452
Reported by: Matthias Köppe
Reviewer(s): Giacomo Pope, Matthias Köppe
  • Loading branch information
Release Manager committed Mar 29, 2024
2 parents 3d59d9c + 9914a6c commit d890829
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 9 deletions.
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
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
14 changes: 14 additions & 0 deletions 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"

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
]
ignore = [
"E501", # Line too long - hard to avoid in doctests, and better handled by black.
]

0 comments on commit d890829

Please sign in to comment.