lint: replace `isort` with Ruff's rule I

[Borda]

Description

since #15312 moved formatting from Black to Rufft, it would make sense also unify import formatting under the same ruff so use build-in I rule instead of additional isort

Checklist

• [x] I am familiar with the Contributing Guidelines.
• [ ] New or existing tests cover these changes.
• [ ] The documentation is up to date with these changes.

[copy-pr-bot[bot]]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[bdice]

Hi @Borda, thanks for this. However, we have been keeping isort because ruff does not support Cython code but isort does. See this discussion for more info: https://github.com/rapidsai/cudf/issues/14882#issuecomment-1910887340

[vyasr]

Hi @Borda, thanks for this. However, we have been keeping isort because ruff does not support Cython code but isort does. See this discussion for more info: #14882 (comment)

@bdice the rest of the discussion in that thread does suggest that we'd be OK with making the switch and losing the limited Cython support. Maybe should go ahead with this now.

[bdice]

You’re right. I forgot the rest of that discussion. @vyasr If you’re in support, I am fine with it.

[vyasr]

/ok to test

[vyasr]

You'll notice that we have isort configuration in each of our Python package's pyproject.toml files, like cudf's here. We definitely want to preserve those rules, so you'll need to add those back as well. https://github.com/rapidsai/cudf/issues/14882#issuecomment-1910887340 discusses a bit how we could accomplish this in a way that shares most of the logic between the different packages in the repo. ruff's docs also have some discussion of how to ensure near-equivalent results, and if you scroll down you'll see discussion of the first/third-party configuration and more. Hopefully that provides you a good enough starting point, but please follow up if you have more questions!

[vyasr]

@Borda let us know if you need any help here. Please also retarget this PR at branch-24.12, we are on the verge of freezing 24.10.

[Borda]

Please also retarget this PR at branch-24.12, we are on the verge of freezing 24.10.

sure will do it over weekend 🦩

[Borda]

Please also retarget this PR at branch-24.12, we are on the verge of freezing 24.10.

sure will do it over weekend 🦩

@vyasr rebased and updated (set I rule and run pre-commit on all files)

[wence-]

Thanks, the next step here is to migrate the subproject-specific configuration for each python package, e.g. for cudf itself here https://github.com/rapidsai/cudf/blob/3420c71cb72f63db8d63164446cca042f354a08e/python/cudf/pyproject.toml#L84-L126 and replace it with the relevant project-specific [tool.ruff.lint.isort] configuration that mimics, as closely as possible, the settings. For example for the cudf_polars subproject, we have already done this, and you can see the classification here https://github.com/rapidsai/cudf/blob/3420c71cb72f63db8d63164446cca042f354a08e/python/cudf_polars/pyproject.toml#L172-L187

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[Borda]

@wence- just did and run applied to all files but seems that triggered some more formatting changes that were unintended 😕

[wence-]

/ok to test

[wence-]

@wence- just did and run applied to all files but seems that triggered some more formatting changes that were unintended 😕

Thanks. It's strange, because almost all the changes are not in the import lists, but other formatting changes. So somehow the application of isort rules in ruff has changed what formatting it applied. Odd.

question: Did you run with pre-commit run --all-files or did you use a locally installed version of ruff to run the checks? If the latter, I think you would need to make sure the versions match up with what we specify in pre-commit.

[wence-]

@wence- just did and run applied to all files but seems that triggered some more formatting changes that were unintended 😕

Thanks. It's strange, because almost all the changes are not in the import lists, but other formatting changes. So somehow the application of isort rules in ruff has changed what formatting it applied. Odd.

question: Did you run with pre-commit run --all-files or did you use a locally installed version of ruff to run the checks? If the latter, I think you would need to make sure the versions match up with what we specify in pre-commit.

Ah, I know what's going on. By introducing a ruff-specific section in the package-specific pyproject.toml files, we're now no longer picking up the "global" configuration (see https://docs.astral.sh/ruff/configuration/#config-file-discovery) for all the details.

tl;dr: the individual pyproject.toml files need to get an additional

[tool.ruff]
extend = "../../pyproject.toml"

[Borda]

Did you run with pre-commit run --all-files

yes that is what I did

[wence-]

Did you run with pre-commit run --all-files

yes that is what I did

Thanks, I think therefore it must be needing to include the "parent" configuration.

[KyleFromNVIDIA]

@Borda Could you please resolve the merge conflicts?

[Borda]

Could you please resolve the merge conflicts?

@KyleFromNVIDIA updated the pyproject.toml files and merged default branch

[KyleFromNVIDIA]

/ok to test

[KyleFromNVIDIA]

/ok to test

[bdice]

@Borda I'm going to trigger tests to run -- we can hold off on merging the upstream for now, unless there are conflicts.

[bdice]

/ok to test

[bdice]

/ok to test

[bdice]

/ok to test

[bdice]

/merge

[wence-]

Thanks @Borda!

[vyasr]

Thanks for this @Borda!