Apply import formatter isort

[akihironitta]

🚀 Refactoring

As isort has been added to ci in #4242, we now need to apply the formatter step by step i.e. a submodule per PR (recommended in #4242 (comment) by @Borda)

Steps

For each PR:

1. choose one submodule from below list and apply isort to it
2. remove the corresponding line in pyproject.toml
3. make sure isort --check passes

Progress

The rest of the submodules should be done by @arnaudgelas.

The followings are PL submodules.

• pytorch_lightning/*.py >> Apply import formatting to files in the 2nd top level #4717
• pytorch_lightning/accelerators/
• pytorch_lightning/callbacks/
• pytorch_lightning/cluster_environments/
• pytorch_lightning/core/
• pytorch_lightning/distributed/
• pytorch_lightning/loggers/
• pytorch_lightning/metrics/
• pytorch_lightning/overrides/
• pytorch_lightning/plugins/
• pytorch_lightning/profiler/
• pytorch_lightning/trainer/
• pytorch_lightning/tuner/
• pytorch_lightning/utilities/

The followings are tests.

• tests/*.py >> Apply import formatting to files in the 2nd top level #4717
• tests/backends/ >> Fix pre-commit isort failure on tests/backends/*.py #5430
• tests/base/ >> Fix pre-commit isort failure on tests/base/*.py #5429
• tests/callbacks/ >> Fix pre-commit isort failure on tests/callbacks/*.py #5428
• tests/checkpointing/ >> Fix pre-commit isort failure on tests/checkpointing/*.py #5427
• tests/core/ >> Fix pre-commit isort failure on tests/core/*.py #5426
• tests/loggers/ >> Fix pre-commit isort failure on tests/loggers/*.py #5425
• tests/plugins/ >> Fix pre-commit isort failure on tests/plugins/*.py #5422
• tests/metrics/ >> Fix pre-commit isort failure on tests/metrics/*.py #5424
• tests/models/ >> Fix pre-commit isort failure on tests/models/*.py #5423
• tests/trainer/ >> Fix pre-commit isort failure on tests/trainer/*.py #5421
• tests/tuner/
• tests/utilities/ >> Fix pre-commit isort failure on tests/utilities/*.py #5420

The followings are other python files.

• *.py (CI: Added isort import check for the code on pull-request #4242)
• docs/ (CI: Added isort import check for the code on pull-request #4242)
• pl_examples/ (Apply isort to pl_examples/ #5291)

[akihironitta]

Would you consider adding the following labels to this issue so that new contributors can work on this issue...?
good first issue
Refactors and code health
Priority P2
tests / CI

[rohitgr7]

it's a pre-commit hook right? wouldn't it be done automatically when someone makes changes to a file in a PR just before it gets merged??

otherwise, I would suggest doing it all at once in a single PR just before the next release (v1.1, I suppose).

[akihironitta]

@rohitgr7 Yes, #4242 added isort to pre-commit as well as to CI testing, but I guess that not everyone is using pre-commit...

Also, #4242 actually tried to apply isort to all files at once, but I think we agreed to split the PR to avoid unexpected errors as @Borda mentioned in the PR:

we just need to make it in smaller steps, like per PL/subpackage...
having one huge change can overlook some minor issues and in general, it is a recommended strategy by williamFalcon to have PRs easier to check and maintain, also it breaks all waiting PRs...

[rohitgr7]

but I guess that not everyone is using pre-commit

oh okay, I thought pre-commit hook runs before a PR get's merged. Is there something like a pre-merge hook which always runs on the changed files before a PR get's merged. If there is one I feel it will be useful to avoid any import formatting issues in the future.

[Borda]

Is there something like a pre-merge hook which always runs on the changed files before a PR get's merged. If there is one I feel it will be useful to avoid any import formatting issues in the future.

yes, that is the CI as @akihironitta mentioned, but at this moment we have almost all files ignored so when a file is fixed we remove it from the ignore lost and since that time it shall be checked with each PR before merge...

[rohitgr7]

Oh, got it. So it will fail that check if imports are not sorted correctly.

Actually I was thinking something like, if possible, could be added that won't fail the check just because the PR author doesn't sorted the imports, but something like a force isort command that will be applied to the changed files before the PR gets merged on master. Only if it's possible obviously.

EDIT: pls ignore the above statements :]

[hassiahk]

Happy to help with this, if more helping hands are needed.

[rohitgr7]

sure go ahead. @hassiahk

[hassiahk]

@rohitgr7, I will start from the bottom utilities so that I don't clash with @akihironitta, since I think @akihironitta is working on the top sections.

[akihironitta]

@hassiahk As the isort config may change in #4960, would you mind waiting for #4960 to be closed?

[hassiahk]

@akihironitta sure, definitely.

[akihironitta]

As #4960 was closed, we all can start to work on this issue :) cc: @hassiahk

[arnaudgelas]

I have all commits ready to fix isort (~20). Should I create 20 PRs for that?
Here is the first one: #5398

[awaelchli]

I noticed that isort and black change import formatting differently, which leads to unnecessary file changes. Maybe consider this configuration: https://black.readthedocs.io/en/stable/compatible_configs.html#isort

[akihironitta]

We are actually introducing yapf instead of black in Bolts since yapf is highly configurable as discussed with @Borda in Lightning-Universe/lightning-bolts#482. We plan to first try yapf in Bolts and to apply it to PL in the future, so we might not need to update the config now.

[akihironitta]

Once @arnaudgelas updates all the PRs, I'll start to check them and mark them as ready-to-go. #5420 (comment) https://github.com/PyTorchLightning/pytorch-lightning/pulls?q=is%3Apr+is%3Aopen+isort

[akihironitta]

Once @arnaudgelas updates all the PRs, I'll start to check them and mark them as ready-to-go. #5420 (comment) https://github.com/PyTorchLightning/pytorch-lightning/pulls?q=is%3Apr+is%3Aopen+isort

@Borda I just realised that I'm not authorised to relabel issues/PRs in PL repo, so if you'd like me to do so, add me to the team on GitHub :]