[infra] replace pycln with ruff #1485
Conversation
pyproject.toml
Outdated
| module = "tenacity.*" | ||
| ignore_missing_imports = true | ||
|
|
||
| [[tool.mypy.overrides]] |
There was a problem hiding this comment.
Not important, but just out of curiosity, is there a specific reason that we are not specifying modules in an array (e.g. here)? I can see pytest.* is declared twice in the file. I assume using a more compact syntax would be helpful?
There was a problem hiding this comment.
this is generated automatically... im actually not sure why its here and why its needed
There was a problem hiding this comment.
theres 229 entries of tool.mypy.overrides in pyproject.toml
and 6 for module = "sqlalchemy.*"
i feel like this is misconfigured
There was a problem hiding this comment.
I see, I think we can leave it for another working item. BTW, here is a stackoverflow I found for what this ignore_missing_imports is used for in case you are interest.
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
I'm all for merging different tools into a single one (and I like ruff a lot). The PR looks misleading since we're also bumping other things like mypy in the same go. I'd prefer to keep them separate in the future to make the reviews easier, but we can combine this one since you already did the work for it.
agreed, i removed |
kevinjqliu
force-pushed
the
kevinjqliu/pre-commit
branch
from
7f1720d to
8f15d8a
Compare
| rev: v0.8.6 | ||
| hooks: | ||
| - id: ruff | ||
| args: [ --fix, --exit-non-zero-on-fix, --preview ] |
There was a problem hiding this comment.
I think we have the --exit-non-zero-on-fix in there for the CI
There was a problem hiding this comment.
I agree with removing --preview: https://docs.astral.sh/ruff/preview/
There was a problem hiding this comment.
added back --exit-non-zero-on-fix
| "UP", # pyupgrade | ||
| ] | ||
| ignore = ["E501","E203","B024","B028","UP037"] | ||
| ignore = ["E501","E203","B024","B028","UP037", "UP035", "UP006"] |
There was a problem hiding this comment.
UP006 makes sense to me, we can do a follow up to fix UP035: https://docs.astral.sh/ruff/rules/deprecated-import/
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
Some small nits, but apart from that, this looks great! Thanks for fixing this @kevinjqliu. Let me approve this to unblock the other PRs
| {file = "jsonpath_ng-1.7.0-py2-none-any.whl", hash = "sha256:898c93fc173f0c336784a3fa63d7434297544b7198124a68f9a3ef9597b0ae6e"}, | ||
| {file = "jsonpath_ng-1.7.0-py3-none-any.whl", hash = "sha256:f3d7f9e848cba1b6da28c55b1c26ff915dc9e0b1ba7e752a53d6da8d5cbd00b6"}, |
There was a problem hiding this comment.
Do we want to revert these as well?
# Rationale for this change This PR removes [tool.black] from pyproject.toml. This is an unused config since formatting now lives in [ruff.toml](https://github.com/apache/iceberg-python/blob/main/ruff.toml). Also, updates contributing.md to reference ruff instead of black. This PR removes dead config from pyproject.toml: - [tool.black] - replaced by ruff-format #127 - [tool.pycln] - replaced by ruff #1485 Also updates contributing.md to reference ruff instead of black. ## Are these changes tested? Yes ## Are there any user-facing changes? No
3 participants
pyclnhas an issue in CI , also see hadialqattan/pycln#249Since we already use ruff linter, this PR replaces
pyclnwithruffCommits:
pre-commit autoupdate(d621712)pycln(230a747)make lint(1cb37db)