microsoft
Should we add isort to the pre-commit?
isort helps to organize the order of dependencies import, the current code is not formatted with isort. If a new PR is formatted with isort, there will be many changes in the order of dependencies import. The reordered code looks nice, but it also adds more work for the reviewers.
What about we use a PR to reformat the code, and also add isort to the pre-commit hooks?
Is it a common or rule-of-thumb practice to use isort? Do you know any other famous repo that also uses isort?
It's quite common in python projects in general. When we talk about packages it's not that common. From what I've found modin - https://github.com/modin-project/modin/blob/master/environment-dev.yml river - https://github.com/online-ml/river/blob/main/pyproject.toml anomalib - https://github.com/openvinotoolkit/anomalib/blob/main/.pre-commit-config.yaml albumentations - https://github.com/albumentations-team/albumentations/blob/master/.pre-commit-config.yaml optuna - https://github.com/optuna/optuna/blob/master/pyproject.toml
As an alternative you could enable ruff version of isort and use it.
It's quite common in python projects in general. When we talk about packages it's not that common. From what I've found modin - https://github.com/modin-project/modin/blob/master/environment-dev.yml river - https://github.com/online-ml/river/blob/main/pyproject.toml anomalib - https://github.com/openvinotoolkit/anomalib/blob/main/.pre-commit-config.yaml albumentations - https://github.com/albumentations-team/albumentations/blob/master/.pre-commit-config.yaml optuna - https://github.com/optuna/optuna/blob/master/pyproject.toml
As an alternative you could enable ruff version of isort and use it.
What do you think? @sonichi @qingyun-wu
If this one will get a go from the core, I can do a PR
Hi @glevv , are you still interested in this? Thanks.
If this one will get a go from the core, I can do a PR
Hi @glevv , are you still interested in this? Thanks.
Yes, I can do this, but I need to know which route are we taking here: ruff version of isort (it will only require change in the config files) or original isort (it will be a new dev dependency)?
Either way it will be a change in the whole codebase (imports sort) and change in the pre-commit hook.
If this one will get a go from the core, I can do a PR
Hi @glevv , are you still interested in this? Thanks.
Yes, I can do this, but I need to know which route are we taking here: ruff version of isort (it will only require change in the config files) or original isort (it will be a new dev dependency)?
Either way it will be a change in the whole codebase (imports sort) and change in the pre-commit hook.
imports sort in this PR is what we wanted, so we won't see a lot imports sort in new PRs.
ruff version sounds better.
A quick question, what do you mean with original isort need a new dev dependency? Do you mean we need to add isort into the dev virtual env? Wouldn't ruff version also install isort?
Thanks.
@thinkall Thank you for the reply.
That's the beauty of ruff: isort rules are integrated in it, so there won't be any new dependencies.
I'll make a PR this week.
In this note, can we enable/install his bot - https://github.com/marketplace/pre-commit-ci - so contributions like this will only update the configuration and the bot will perform the changes so it is clear what are the automated changes and what is additional enhancement...
