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

[Feature]: Expand ruff checkers #495

Open
boxydog opened this issue Oct 20, 2024 · 1 comment
Open

[Feature]: Expand ruff checkers #495

boxydog opened this issue Oct 20, 2024 · 1 comment

Comments

@boxydog
Copy link

boxydog commented Oct 20, 2024

Why is this issue important?

ruff does a good job of checking and auto-fixing python to be more correct and more consistent.

This project is just using the default 4 checkers, but there are dozens. One I like is UP, like pyupgrade, that updates all the code to be most idiomatic for our chosen python version (e.g., python 3.12). As an example, I noticed a bunch of Optional[str] type hints, which is now str | None.

Another which is aggressive but helpful is "Q" just to make sure all quotes are the same (default: double quotes). This is the kind of tiny stuff that it's good to just have a machine do.

In combination with pre-commit, ruff can auto-rewrite our code safely and with no effort as we push stuff up.

Current State

Default checkers.

Expected State

More checkers.

Implementation Plan

  • Add more checkers to the tool.ruff.lint section of pyproject.toml in python.
  • If there are errors that are not auto-fixed, add them to the ignore list.
  • For those checkers with only a few errors, try removing them from the ignore list and fixing them by hand.

Relevant Code Snippets

# Below is an example I have from another project

[tool.ruff.lint]
# see https://docs.astral.sh/ruff/configuration/#using-pyprojecttoml
select = [
  # default Ruff checkers as of ruff 0.3.4: E4, E7, E9, F
  "E4",
  "E7",
  "E9",
  # "F" contains autoflake, see https://github.com/astral-sh/ruff/issues/1647
  "F",  # pyflakes

  # TODO: "A",   # flake8-builtins
  # TODO: "ARG", # flake8-unused-arguments
  "B",   # flake8-bugbear
  "BLE", # flake8-blind-except
  # TODO: Do I want "COM", # flake8-commas
  "C4",  # flake8-comprehensions
  # TODO: "DTZ", # flake8-datetimez
  # TODO: "EM",  # flake8-errmsg
  "EXE", # flake8-executable
  # TODO: "FURB", # refurb
  # TODO: "FBT", # flake8-boolean-trap
  # TODO: "G",   # flake8-logging-format
  "I",   # isort
  "ICN", # flake8-import-conventions
  "INP", # flake8-no-pep420
  # TODO: "INT", # flake8-gettext
  "ISC", # flake8-implicit-str-concat
  # TODO: "LOG", # flake8-logging
  "PERF", # perflint
  "PIE", # flake8-pie
  "PL",  # pylint
  "PYI", # flake8-pyi
  # TODO: "RET", # flake8-return
  "RSE", # flake8-raise
  "RUF",
  # TODO: "SIM", # flake8-simplify
  "SLF", # flake8-self
  "SLOT", # flake8-slots
  "TID", # flake8-tidy-imports
  "UP",  # pyupgrade
  "Q",   # flake8-quotes
  "TCH", # flake8-type-checking
  "T10", # flake8-debugger
  "T20", # flake8-print
  # TODO: "S",   # flake8-bandit
  "YTT", # flake8-2020
  # TODO: add more flake8 rules
]

ignore = [
  "ISC001",  # ruff formatter thinks we should ignore this one
  "PLR2004",
  "INP001",
  "T201",
  "RUF012",
  "SLF001",
  "PLR0913",
  "PLR0912",
  "PLW2901",
  "B904",
  "PLR0915",
  "RUF015",
  # I don't like RUF005, it looks less readable to me, and the performance improvement
  # is likely unimportant.
  "RUF005",
]
@boxydog
Copy link
Author

boxydog commented Oct 20, 2024

For example, just adding "I", "UP", and "SIM" on my branch produced this:

$ ruff check --fix
src/functions/create_archive.py:88:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements
src/functions/generate_treasury_report.py:457:9: SIM118 Use `key in dict` instead of `key in dict.keys()`
src/functions/subrecipient_treasury_report_gen.py:135:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements
src/functions/validate_workbook.py:34:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements
tests/conftest.py:32:12: SIM115 Use context handler for opening files
tests/conftest.py:231:12: SIM115 Use context handler for opening files
Found 318 errors (312 fixed, 6 remaining).
No fixes available (2 hidden fixes can be enabled with the `--unsafe-fixes` option).

$ git diff | wc -l
    2121

A lot of it is just re-ordering imports, and using a newer style of type hinting.

This is stuff we'd like a computer to keep straight for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant