From 1fc8581678ffd2bad8736d1dfe113cb3473dea36 Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Fri, 24 Jan 2020 10:21:14 +0000 Subject: [PATCH 1/7] Add isort to CI --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- .pre-commit-config.yaml | 12 ++++----- azure-pipelines.yml | 12 +++++++++ ci/min_deps_check.py | 1 + ci/requirements/py36-min-all-deps.yml | 1 + ci/requirements/py36.yml | 1 + ci/requirements/py37-windows.yml | 1 + ci/requirements/py37.yml | 1 + doc/contributing.rst | 35 ++++++++++++--------------- 9 files changed, 40 insertions(+), 26 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index d78dd38dd85..ac7cb1d5406 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,5 +2,5 @@ - [ ] Closes #xxxx - [ ] Tests added - - [ ] Passes `black . && mypy . && flake8` + - [ ] Passes `black . && mypy . && isort -rc . && flake8` - [ ] Fully documented, including `whats-new.rst` for all changes and `api.rst` for new API diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ed62c1c256e..54dbc62eb40 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ # https://pre-commit.com/ -# https://github.com/python/black#version-control-integration repos: + # https://github.com/python/black#version-control-integration - repo: https://github.com/python/black rev: stable hooks: @@ -14,7 +14,11 @@ repos: rev: v0.761 # Must match ci/requirements/*.yml hooks: - id: mypy - # run these occasionally, ref discussion https://github.com/pydata/xarray/pull/3194 + - repo: https://github.com/timothycrosley/isort + rev: 4.3.21-2 + hooks: + - id: isort + # 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 +27,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 diff --git a/azure-pipelines.yml b/azure-pipelines.yml index d6ee76c7d3f..b09c4ec127c 100644 --- a/azure-pipelines.yml +++ b/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' diff --git a/ci/min_deps_check.py b/ci/min_deps_check.py index a5ba90679b7..527093cf5bc 100755 --- a/ci/min_deps_check.py +++ b/ci/min_deps_check.py @@ -15,6 +15,7 @@ "coveralls", "flake8", "hypothesis", + "isort", "mypy", "pip", "pytest", diff --git a/ci/requirements/py36-min-all-deps.yml b/ci/requirements/py36-min-all-deps.yml index ea707461013..e79e854ecc9 100644 --- a/ci/requirements/py36-min-all-deps.yml +++ b/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 diff --git a/ci/requirements/py36.yml b/ci/requirements/py36.yml index 7450fafbd86..929a36295a0 100644 --- a/ci/requirements/py36.yml +++ b/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 diff --git a/ci/requirements/py37-windows.yml b/ci/requirements/py37-windows.yml index d9e634c74ae..8755ea2cef6 100644 --- a/ci/requirements/py37-windows.yml +++ b/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 diff --git a/ci/requirements/py37.yml b/ci/requirements/py37.yml index 2f879e29f87..dd3267abe4c 100644 --- a/ci/requirements/py37.yml +++ b/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 diff --git a/doc/contributing.rst b/doc/contributing.rst index 3cd0b3e8868..29c8df5188a 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -345,33 +345,31 @@ as possible to avoid mass breakages. Code Formatting ~~~~~~~~~~~~~~~ -Xarray uses `Black `_ and -`Flake8 `_ 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 `_ for standardized code formatting +- `Flake8 `_ for general code quality +- `isort `_ for standardized order in imports. + See also `flake8-isort `_. +- `mypy `_ for static type checking on `type hints + `_ + ``pip``:: - pip install black flake8 + pip install black flake8 isort mypy and then run from the root of the Xarray repository:: - black . + black -t py36 . + isort -rc . 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 `_ performs static type checking, which can - make it easier to catch bugs. Please run ``mypy xarray`` if you annotate any - code with `type hints `_. -- `isort `_ will highlight - incorrectly sorted imports. ``isort -y`` will automatically fix them. See - also `flake8-isort `_. - Optionally, you may wish to setup `pre-commit hooks `_ -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,9 +378,8 @@ 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 -``git commit --no-verify``. +from the root of the xarray repository. +You can skip the pre-commit checks with ``git commit --no-verify``. .. note:: From 58a2755a546645ee22818ff108edc659d04fab7d Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Fri, 24 Jan 2020 10:39:13 +0000 Subject: [PATCH 2/7] Run isort before black --- doc/contributing.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index 29c8df5188a..6735e0ede2f 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -360,8 +360,8 @@ xarray uses several tools to ensure a consistent code format throughout the proj and then run from the root of the Xarray repository:: - black -t py36 . isort -rc . + black -t py36 . flake8 mypy . @@ -378,8 +378,8 @@ and then running:: pre-commit install -from the root of the xarray repository. -You can skip the pre-commit checks with ``git commit --no-verify``. +from the root of the xarray repository. You can skip the pre-commit checks with +``git commit --no-verify``. .. note:: From bc0b96ab3df11fec851f3366928a8af104120333 Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Fri, 24 Jan 2020 10:40:07 +0000 Subject: [PATCH 3/7] Run isort before black --- .pre-commit-config.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 54dbc62eb40..57a987faa20 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,5 +1,10 @@ # https://pre-commit.com/ 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 @@ -14,10 +19,6 @@ repos: rev: v0.761 # Must match ci/requirements/*.yml hooks: - id: mypy - - repo: https://github.com/timothycrosley/isort - rev: 4.3.21-2 - hooks: - - id: isort # run this occasionally, ref discussion https://github.com/pydata/xarray/pull/3194 # - repo: https://github.com/asottile/pyupgrade # rev: v1.22.1 From ec997b38ede93ea6651dedea65b9385ebcfaf75f Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Fri, 24 Jan 2020 10:42:36 +0000 Subject: [PATCH 4/7] What's new --- doc/whats-new.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index c310d53190d..1065c0a26cc 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -144,6 +144,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 `_ +- Use of isort is now enforced by CI. + (:pull:`3721`) by `Guido Imperiale `_ v0.14.1 (19 Nov 2019) From e185268a8dc7b6a7a4e424ff470ff9d82dd87695 Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Fri, 24 Jan 2020 10:43:18 +0000 Subject: [PATCH 5/7] Run isort before black --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index ac7cb1d5406..61d8671ab48 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,5 +2,5 @@ - [ ] Closes #xxxx - [ ] Tests added - - [ ] Passes `black . && mypy . && isort -rc . && flake8` + - [ ] Passes `isort -rc . && black . && mypy . && && flake8` - [ ] Fully documented, including `whats-new.rst` for all changes and `api.rst` for new API From 05e79fed5fd65b4d414568bca85f120ca3a6c983 Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Fri, 24 Jan 2020 10:44:37 +0000 Subject: [PATCH 6/7] DEMO: deliberate isort failure --- xarray/core/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/core/common.py b/xarray/core/common.py index e908c69dd14..23629dce793 100644 --- a/xarray/core/common.py +++ b/xarray/core/common.py @@ -3,8 +3,8 @@ from html import escape from textwrap import dedent from typing import ( - Any, Callable, + Any, Dict, Hashable, Iterable, From dda2b296737f0f5236b8644de46b45e05009b64a Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Fri, 24 Jan 2020 10:56:08 +0000 Subject: [PATCH 7/7] Fix isort failure --- xarray/core/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/core/common.py b/xarray/core/common.py index 23629dce793..e908c69dd14 100644 --- a/xarray/core/common.py +++ b/xarray/core/common.py @@ -3,8 +3,8 @@ from html import escape from textwrap import dedent from typing import ( - Callable, Any, + Callable, Dict, Hashable, Iterable,