Pytorch-lightning: 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 https://github.com/PyTorchLightning/pytorch-lightning/pull/4242#issuecomment-724323170 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 followings are PL submodules.

• [x] pytorch_lightning/*.py (#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.

• [x] tests/*.py (#4717)
• [ ] tests/backends/
• [ ] tests/base/
• [ ] tests/callbacks/
• [ ] tests/checkpointing/
• [ ] tests/core/
• [ ] tests/loggers/
• [ ] tests/plugins/
• [ ] tests/metrics/
• [ ] tests/models/
• [ ] tests/trainer/
• [ ] tests/tuner/
• [ ] tests/utilities/

The followings are other python files.

• [x] *.py (#4242)
• [x] docs/ (#4242)
• [ ] pl_examples/ (small enough to apply in one PR?)

[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.