CI: Added isort import check for the code on pull-request (#4242)

* added isort CI job and updated isort config

* changed CI check output from files to full diff

* added isort pre-commit hook

* Added missing first party and restricted files affected by isort

* Applied isort to root-level, docs and benchmarks

* Apply suggestions from code review

Co-authored-by: Nathan Painchaud <nathanpainchaud@gmail.com>
Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
Co-authored-by: chaton <thomas@grid.ai>

.github/workflows/code-formatting.yml

@@ -7,6 +7,21 @@ on:  # Trigger the workflow on push or pull request, but only for the master bra
     branches: [master, "release/*"]
 
 jobs:
+  imports-check-isort:
+    name: Check valid import formatting with isort
+    runs-on: ubuntu-20.04
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+      - name: Set up Python 3.8
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.8
+      - name: Install isort
+        run: pip install isort==5.6.4
+      - name: Run isort
+        run: isort --settings-path=./pyproject.toml --check-only --diff .
+
   code-black:
     name: Check code formatting with Black
     runs-on: ubuntu-20.04

.pre-commit-config.yaml

@@ -21,3 +21,12 @@ repos:
     hooks:
       - id: trailing-whitespace
       - id: end-of-file-fixer
+
+  - repo: local
+    hooks:
+      - id: isort
+        name: isort
+        entry: python -m isort
+        args: [--settings-path, ./pyproject.toml]
+        language: system
+        types: [python]

benchmarks/test_parity.py

@@ -6,7 +6,7 @@
 
 import tests.base.develop_utils as tutils
 from pytorch_lightning import Trainer, seed_everything
-from tests.base.models import ParityModuleRNN, ParityModuleMNIST
+from tests.base.models import ParityModuleMNIST, ParityModuleRNN
 
 
 @pytest.mark.parametrize('cls_model,max_diff', [

docs/source/conf.py

@@ -12,14 +12,14 @@
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 
-import os
-import sys
+# import m2r
+import builtins
 import glob
-import shutil
 import inspect
+import os
+import shutil
+import sys
 
-# import m2r
-import builtins
 import pt_lightning_sphinx_theme
 
 PATH_HERE = os.path.abspath(os.path.dirname(__file__))

pyproject.toml

@@ -13,3 +13,19 @@ ignore = ["W504", "W504", "E402", "E731", "C40", "E741", "F40", "F841"]
 line-length = 120
 target-version = ["py38"]
 exclude = "(.eggs|.git|.hg|.mypy_cache|.nox|.tox|.venv|.svn|_build|buck-out|build|dist)"
+
+[tool.isort]
+known_first_party = [
+    "bencharmks",
+    "docs",
+    "pl_examples",
+    "pytorch_lightning",
+    "tests",
+]
+skip_glob = [
+    "pl_examples/*",
+    "pytorch_lightning/*",
+    "tests/*"
+]    # Only apply formatting on root-level scripts, docs and benchmarks
+profile = "black"
+line_length = 120

requirements/test.txt

@@ -9,6 +9,7 @@ check-manifest
 twine==1.13.0
 scikit-image>=0.17.2
 black>=20.8b1
+isort>=5.6.4
 pre-commit>=1.0
 
 cloudpickle>=1.2

setup.cfg

@@ -90,8 +90,3 @@ convention = pep257
 # D202: Ignore a blank line after docstring (collision with Python Black in decorators)
 add-ignore = D104,D107,D202
 max-line-length = 120
-
-[tool:isort]
-known_first_party = pytorch_lightning,tests
-default_section=THIRDPARTY
-line_length=120

setup.py

@@ -15,8 +15,9 @@
 
 import os
 from io import open
+
 # Always prefer setuptools over distutils
-from setuptools import setup, find_packages
+from setuptools import find_packages, setup
 
 try:
     import builtins