-
Notifications
You must be signed in to change notification settings - Fork 174
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
Use Ruff to sort imports #3230
Use Ruff to sort imports #3230
Conversation
.pre-commit-config.yaml
Outdated
@@ -42,7 +42,7 @@ repos: | |||
rev: v0.8.3 | |||
hooks: | |||
- id: ruff | |||
args: ["--fix", "--show-fixes"] | |||
args: ["--fix", "--show-fixes", "--extend-select=I"] |
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.
Ideally, this is configured in a config file (such as pyproject.toml
), rather than baked into the command in this way.
But, if I add extend-select = ["I"]
to the top-level pyproject.toml
file instead of this change, it doesn't work (no import sorting is performed). I'm not sure why that's the case.
On the other hand, extend-select=["I"]
to the individual projects' pyproject.toml
, such as cuda_parallel/
or cuda_experimental/
does work, but that wouldn't work to sort imports of the other .py
files in the project.
Any suggestions? @bdice perhaps you might know?
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.
I think there are some funny rules about how extend options are inherited? Try using a normal select instead of extend-select, or try using the “extend” option to control inheritance: https://github.com/rapidsai/cudf/blob/88d925187566cf187a9a4d5c4619e00e2a9fb9b1/python/cudf/pyproject.toml#L145
Also I think the “fix” (definitely) and “show fixes” (possibly) options can also be put into the config instead of passed as an arg here. I recommend keeping the pre-commit configuration argument-free so that running ruff on its own does the same thing as pre-commit. https://github.com/rapidsai/cudf/blob/88d925187566cf187a9a4d5c4619e00e2a9fb9b1/python/cudf_polars/pyproject.toml#L86
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.
Thanks! This fixed it:
try using the “extend” option to control inheritance
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.
By default ruff only uses a single pyproject.toml and it'll the "closest" one to the files that you are modifying and performs no extending of settings, so if you put settings into subproject pyproject.toml files it will completely override the settings from the parent directory's pyproject.toml. The extend option is how you tell it to merge them. ruff will ignore any pyproject.toml files that don't have a ruff section, which is why you still get proper linting with multiple pyproject.toml files but only a higher-level one containing a ruff config.
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.
🟩 CI finished in 53m 56s: Pass: 100%/176 | Total: 1d 05h | Avg: 9m 54s | Max: 47m 29s | Hits: 76%/22522
|
Project | |
---|---|
+/- | CCCL Infrastructure |
+/- | libcu++ |
CUB | |
+/- | Thrust |
CUDA Experimental | |
+/- | python |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
+/- | CCCL Infrastructure |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 176)
# | Runner |
---|---|
125 | linux-amd64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
10 | linux-arm64-cpu16 |
1 | linux-amd64-gpu-h100-latest-1-testing |
8bc8bb0
to
bf3e97e
Compare
bf3e97e
to
5951ee2
Compare
Sorry I just noticed now that we have a top level pyproject.toml... it's a bit nerve wrecking since there's no installable packages. |
This is fine — and becoming increasingly common, as pyproject.toml can be used solely for configuration of various tools, with no package definition. We have repo-wide pyproject.toml files in RAPIDS and other projects. |
🟩 CI finished in 1h 03m: Pass: 100%/176 | Total: 1d 03h | Avg: 9m 12s | Max: 57m 34s | Hits: 85%/22522
|
Project | |
---|---|
+/- | CCCL Infrastructure |
+/- | libcu++ |
CUB | |
+/- | Thrust |
CUDA Experimental | |
+/- | python |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
+/- | CCCL Infrastructure |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 176)
# | Runner |
---|---|
125 | linux-amd64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
10 | linux-arm64-cpu16 |
1 | linux-amd64-gpu-h100-latest-1-testing |
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.
Python changes LGTM!
(@shwina Would you mind propagating the same setup to cuda-python repo after this is merged? 🙂)
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.
Approving for libcudacxx changes.
Description
This PR updates the ruff configuration to include sorting of imports. There are also small updates to the
pyproject.toml
files of thecuda_parallel
andcuda_cooperative
packages. The configuration update is in 401ed43.In addition, following @bdice's suggestion here, I've made it so that:
This is done in 4594224.
Checklist