-
Notifications
You must be signed in to change notification settings - Fork 242
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 ruff support #1700
Add ruff support #1700
Conversation
pyproject.toml
Outdated
indent-width = 4 | ||
|
||
# Assume Python 3.8 | ||
target-version = "py38" |
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.
Would this cause any trouble? I think the version depends on user's system.
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.
According to astral-sh/ruff#2519 (comment), is ok to indicate the minimum version, which according to setup.py
is 3.9
, so I added it in b2292ee
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.
In https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md#015 it's recommended to use project.requires-python
. It's used in ruff here.
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.
Changed in 442353e
combine_as_imports = true | ||
multi_line_output = 3 | ||
skip=docs | ||
|
||
[tool:pytest] |
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.
Do you think that we can also move pytest config to the pyproject file. I saw it is supported here. (not necessary in this PR though)
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.
Indeed! I want to do this in another PR. In addition, I wanna add pre-commit hooks :) (also in a different PR). I want to work on batches of small changes.
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.
This is super helpful! Thanks @juanitorduz!
I will continue with other PRs to remove setup.cfg and add pre-commit hooks :) |
The change seems to cause the master branch to fail. Could you take a look? |
* add ruff * fix line-length * min python version * add requires-python section
This PR proposes adding ruff as a linter and formater. See pyro-ppl/pyro#3231
I made the configuration very explicit, see https://github.com/astral-sh/ruff?tab=readme-ov-file#configuration
Just very minor changes to the code style were needed.
Locally,
make lint
andmake format
work as expected.