Add isort to CI (#3721)

* Add isort to CI

* Run isort before black

* Run isort before black

* What's new

* Run isort before black

* DEMO: deliberate isort failure

* Fix isort failure

.github/PULL_REQUEST_TEMPLATE.md

@@ -2,5 +2,5 @@
 
  - [ ] Closes #xxxx
  - [ ] Tests added
- - [ ] Passes `black . && mypy . && flake8`
+ - [ ] Passes `isort -rc . && black . && mypy . && && flake8`
  - [ ] Fully documented, including `whats-new.rst` for all changes and `api.rst` for new API

.pre-commit-config.yaml

@@ -1,6 +1,11 @@
 # https://pre-commit.com/
-# https://github.com/python/black#version-control-integration
 repos:
+  # isort should run before black as black sometimes tweaks the isort output
+  - repo: https://github.com/timothycrosley/isort
+    rev: 4.3.21-2
+    hooks:
+      - id: isort
+  # https://github.com/python/black#version-control-integration
   - repo: https://github.com/python/black
     rev: stable
     hooks:
@@ -14,7 +19,7 @@ repos:
     rev: v0.761  # Must match ci/requirements/*.yml
     hooks:
       - id: mypy
-  # run these occasionally, ref discussion https://github.com/pydata/xarray/pull/3194
+  # run this occasionally, ref discussion https://github.com/pydata/xarray/pull/3194
   # - repo: https://github.com/asottile/pyupgrade
   #   rev: v1.22.1
   #   hooks:
@@ -23,7 +28,3 @@ repos:
   #         - "--py3-only"
   #         # remove on f-strings in Py3.7
   #         - "--keep-percent-format"
-  # - repo: https://github.com/timothycrosley/isort
-  #   rev: 4.3.21-2
-  #   hooks:
-  #     - id: isort

azure-pipelines.yml

@@ -84,6 +84,18 @@ jobs:
       mypy .
     displayName: mypy type checks
 
+- job: isort
+  variables:
+    conda_env: py37
+  pool:
+    vmImage: 'ubuntu-16.04'
+  steps:
+  - template: ci/azure/install.yml
+  - bash: |
+      source activate xarray-tests
+      isort -rc --check .
+    displayName: isort formatting checks
+
 - job: MinimumVersionsPolicy
   pool:
     vmImage: 'ubuntu-16.04'

ci/min_deps_check.py

@@ -15,6 +15,7 @@
     "coveralls",
     "flake8",
     "hypothesis",
+    "isort",
     "mypy",
     "pip",
     "pytest",

ci/requirements/py36-min-all-deps.yml

@@ -23,6 +23,7 @@ dependencies:
   - hdf5=1.10
   - hypothesis
   - iris=2.2
+  - isort
   - lxml=4.4  # Optional dep of pydap
   - matplotlib=3.1
   - mypy=0.761  # Must match .pre-commit-config.yaml

ci/requirements/py36.yml

@@ -19,6 +19,7 @@ dependencies:
   - hdf5
   - hypothesis
   - iris
+  - isort
   - lxml    # optional dep of pydap
   - matplotlib
   - mypy=0.761  # Must match .pre-commit-config.yaml

ci/requirements/py37-windows.yml

@@ -19,6 +19,7 @@ dependencies:
   - hdf5
   - hypothesis
   - iris
+  - isort
   - lxml    # Optional dep of pydap
   - matplotlib
   - mypy=0.761  # Must match .pre-commit-config.yaml

ci/requirements/py37.yml

@@ -19,6 +19,7 @@ dependencies:
   - hdf5
   - hypothesis
   - iris
+  - isort
   - lxml    # Optional dep of pydap
   - matplotlib
   - mypy=0.761  # Must match .pre-commit-config.yaml

doc/contributing.rst

@@ -345,33 +345,31 @@ as possible to avoid mass breakages.
 Code Formatting
 ~~~~~~~~~~~~~~~
 
-Xarray uses `Black <https://black.readthedocs.io/en/stable/>`_ and
-`Flake8 <http://flake8.pycqa.org/en/latest/>`_ to ensure a consistent code
-format throughout the project. ``black`` and ``flake8`` can be installed with
+xarray uses several tools to ensure a consistent code format throughout the project:
+
+- `Black <https://black.readthedocs.io/en/stable/>`_ for standardized code formatting
+- `Flake8 <http://flake8.pycqa.org/en/latest/>`_ for general code quality
+- `isort <https://github.com/timothycrosley/isort>`_ for standardized order in imports.
+  See also `flake8-isort <https://github.com/gforcada/flake8-isort>`_.
+- `mypy <http://mypy-lang.org/>`_ for static type checking on `type hints
+  <https://docs.python.org/3/library/typing.html>`_
+
 ``pip``::
 
-   pip install black flake8
+   pip install black flake8 isort mypy
 
 and then run from the root of the Xarray repository::
 
-   black .
+   isort -rc .
+   black -t py36 .
    flake8
+   mypy .
 
 to auto-format your code. Additionally, many editors have plugins that will
 apply ``black`` as you edit files.
 
-Other recommended but optional tools for checking code quality (not currently
-enforced in CI):
-
-- `mypy <http://mypy-lang.org/>`_ performs static type checking, which can
-  make it easier to catch bugs. Please run ``mypy xarray`` if you annotate any
-  code with `type hints <https://docs.python.org/3/library/typing.html>`_.
-- `isort <https://github.com/timothycrosley/isort>`_ will highlight
-  incorrectly sorted imports. ``isort -y`` will automatically fix them. See
-  also `flake8-isort <https://github.com/gforcada/flake8-isort>`_.
-
 Optionally, you may wish to setup `pre-commit hooks <https://pre-commit.com/>`_
-to automatically run ``black`` and ``flake8`` when you make a git commit. This
+to automatically run all the above tools every time you make a git commit. This
 can be done by installing ``pre-commit``::
 
    pip install pre-commit
@@ -380,8 +378,7 @@ and then running::
 
    pre-commit install
 
-from the root of the Xarray repository. Now ``black`` and ``flake8`` will be run
-each time you commit changes. You can skip these checks with
+from the root of the xarray repository. You can skip the pre-commit checks with
 ``git commit --no-verify``.
 
 .. note::

doc/whats-new.rst

@@ -157,6 +157,8 @@ Internal Changes
 - Replaced versioneer with setuptools-scm. Moved contents of setup.py to setup.cfg.
   Removed pytest-runner from setup.py, as per deprecation notice on the pytest-runner
   project. (:pull:`3714`) by `Guido Imperiale <https://github.com/crusaderky>`_
+- Use of isort is now enforced by CI.
+  (:pull:`3721`) by `Guido Imperiale <https://github.com/crusaderky>`_
 
 
 v0.14.1 (19 Nov 2019)