-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add pre-commit hooks with ruff formatter/linter rules #26
Conversation
Adding a .pre-commit-config.yaml file with some pre-commit hooks and the ruff linter/formatter. The ruff linter is configured to do autofix, and will run on python scripts and jupyter notebooks.
Enforce certain ruff formatting and lint rules such as UNIX-style line-endings, pycodestyle, pyflakes, isort, numpy, pylint and pyupgrade.
Fix `F841: Local variable `cli` is assigned to but never used` on trainer.py.
for more information, see https://pre-commit.ci
Setting up pre-commit.ci to only run updates quarterly instead of the weekly default. Also explicitly stating that Pull Requests will be autofixed.
[tool.ruff.lint] | ||
# https://docs.astral.sh/ruff/rules/ | ||
select = [ | ||
"E", # pycodestyle errors | ||
"F", # pyflakes | ||
"I", # isort | ||
"NPY", # numpy | ||
"PL", # pylint | ||
"UP", # pyupgrade | ||
"W", # pycodestyle warnings | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if these rules from https://docs.astral.sh/ruff/rules are too much or too little!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me for now. We can always adapt if we want more or someone feels slowed down by any of the linters.
# https://pre-commit.ci/#configuration | ||
ci: | ||
autofix_prs: true | ||
autoupdate_schedule: quarterly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have set-up pre-commit.ci GitHub App to fix lint/formatting errors in Pull Requests by default, but we can disable this if it gets annoying.
Also, pre-commit autoupdate
will be ran quarterly (default is weekly, but that might get noisy).
Extra configs can be found at https://pre-commit.ci/#configuration
@@ -9,7 +9,6 @@ | |||
- https://lightning.ai/docs/pytorch/2.1.0/cli/lightning_cli.html | |||
- https://pytorch-lightning.medium.com/introducing-lightningcli-v2-supercharge-your-training-c070d43c7dd6 | |||
""" | |||
import torch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See example commit 012a7a8 where pre-commit-ci
automatically removed this unused import!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I haven't used pre-commit.ci before. Seems handy!
[tool.ruff.lint] | ||
# https://docs.astral.sh/ruff/rules/ | ||
select = [ | ||
"E", # pycodestyle errors | ||
"F", # pyflakes | ||
"I", # isort | ||
"NPY", # numpy | ||
"PL", # pylint | ||
"UP", # pyupgrade | ||
"W", # pycodestyle warnings | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me for now. We can always adapt if we want more or someone feels slowed down by any of the linters.
Yeah, this is actually my first time setting it up too, but I've been meaning to try it for a while (it's only free for public repos). Gonna merge this in now, thanks @yellowcap for reviewing! |
What I am changing
How I did it
.pre-commit-config.yaml
file for standard pre-commit hookspyproject.toml
configuration file for ruff formatter/linter rulesHow you can test it
pre-commit
and runpre-commit run --all-files
locallyRelated Issues
References: