diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9d4024a69f6..34b9997ba07 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,4 +2,5 @@ - [ ] Closes #xxxx - [ ] Tests added + - [ ] Passes `black .` & `flake8` - [ ] Fully documented, including `whats-new.rst` for all changes and `api.rst` for new API diff --git a/README.rst b/README.rst index f2f754f37c9..53f51392a1a 100644 --- a/README.rst +++ b/README.rst @@ -11,6 +11,9 @@ xarray: N-D labeled arrays and datasets :target: https://pandas.pydata.org/speed/xarray/ .. image:: https://img.shields.io/pypi/v/xarray.svg :target: https://pypi.python.org/pypi/xarray/ +.. image:: https://img.shields.io/badge/code%20style-black-000000.svg + :target: https://github.com/python/black + **xarray** (formerly **xray**) is an open source project and Python package that makes working with labelled multi-dimensional arrays simple, diff --git a/azure-pipelines.yml b/azure-pipelines.yml index ccf8392c5a7..82d3b70704b 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -46,7 +46,7 @@ jobs: steps: - template: ci/azure/unit-tests.yml -- job: Lint +- job: LintFlake8 pool: vmImage: 'ubuntu-16.04' steps: @@ -56,6 +56,16 @@ jobs: - bash: flake8 displayName: flake8 lint checks +- job: FormattingBlack + pool: + vmImage: 'ubuntu-16.04' + steps: + - task: UsePythonVersion@0 + - bash: python -m pip install black + displayName: Install black + - bash: black --check . + displayName: black formatting check + - job: TypeChecking variables: conda_env: py37 diff --git a/doc/contributing.rst b/doc/contributing.rst index 9017c3dde7c..14ecb65f295 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -340,23 +340,24 @@ do not make sudden changes to the code that could have the potential to break a lot of user code as a result, that is, we need it to be as *backwards compatible* as possible to avoid mass breakages. -Python (PEP8) -~~~~~~~~~~~~~ +Code Formatting +~~~~~~~~~~~~~~~ -*xarray* uses the `PEP8 `_ standard. -There are several tools to ensure you abide by this standard. Here are *some* of -the more common ``PEP8`` issues: +Xarray uses `Black `_ and +`Flake8 `_ to ensure a consistent code +format throughout the project. ``black`` and ``flake8`` can be installed with +``pip``:: - - we restrict line-length to 79 characters to promote readability - - passing arguments should have spaces after commas, e.g. ``foo(arg1, arg2, kw1='bar')`` + pip install black flake8 -:ref:`Continuous Integration ` will run -the `flake8 `_ tool -and report any stylistic errors in your code. Therefore, it is helpful before -submitting code to run the check yourself: +and then run from the root of the Xarray repository:: + black . flake8 +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): @@ -367,8 +368,35 @@ enforced in CI): incorrectly sorted imports. ``isort -y`` will automatically fix them. See also `flake8-isort `_. -Note that your code editor probably supports extensions that can show results -of these checks inline as you type. +Optionally, you may wish to setup `pre-commit hooks `_ +to automatically run ``black`` and ``flake8`` when you make a git commit. This +can be done by installing ``pre-commit``:: + + pip install pre-commit + +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``. + +.. note:: + + If you were working on a branch *prior* to the code being reformatted with black, + you will likely face some merge conflicts. These steps can eliminate many of those + conflicts. Because they have had limited testing, please reach out to the core devs + on your pull request if you face any issues, and we'll help with the merge: + + - Merge the commit on master prior to the `black` commit into your branch + `git merge f172c673`. Any conflicts are real conflicts and require resolving + - Apply `black .` to your branch and commit + # TODO: insert after the black commit is on master + - Merge master at the `black` commit, resolving in favor of 'our' changes: + `git merge [master-at-black-commit] -X ours`. You shouldn't have any merge conflicts + - Merge current master `git merge master`; any conflicts here are real and + again require resolving Backwards Compatibility ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/gallery/plot_cartopy_facetgrid.py b/doc/gallery/plot_cartopy_facetgrid.py index 5a1c14d8721..cfa9d3b07ec 100644 --- a/doc/gallery/plot_cartopy_facetgrid.py +++ b/doc/gallery/plot_cartopy_facetgrid.py @@ -33,8 +33,8 @@ col="time", col_wrap=1, # multiplot settings aspect=ds.dims["lon"] / ds.dims["lat"], # for a sensible figsize - subplot_kws={"projection": map_proj}, -) # the plot's projection + subplot_kws={"projection": map_proj}, # the plot's projection +) # We have to set the map's options on all four axes for ax in p.axes.flat: diff --git a/setup.cfg b/setup.cfg index 128550071cc..68fc3b6e06c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -15,22 +15,26 @@ markers = slow: slow tests [flake8] -max-line-length=79 +max-line-length=88 ignore= + # whitespace before ':' - doesn't work well with black + E203 E402 + # do not assign a lambda expression, use a def E731 - E741 + # line break before binary operator W503 - W504 # Unused imports; TODO: Allow typing to work without triggering errors F401 exclude= doc [isort] -default_section=THIRDPARTY -known_first_party=xarray -multi_line_output=4 +multi_line_output=3 +include_trailing_comma=True +force_grid_wrap=0 +use_parentheses=True +line_length=88 # Most of the numerical computing stack doesn't have type annotations yet. [mypy-affine.*] diff --git a/xarray/tests/test_dataarray.py b/xarray/tests/test_dataarray.py index 09546177c65..118b28636dc 100644 --- a/xarray/tests/test_dataarray.py +++ b/xarray/tests/test_dataarray.py @@ -95,8 +95,8 @@ def test_repr_multiindex_long(self): Coordinates: * x (x) MultiIndex - level_1 (x) object 'a' 'a' 'a' 'a' 'a' 'a' 'a' ... 'd' 'd' 'd' 'd' 'd' 'd' - - level_2 (x) int64 1 2 3 4 5 6 7 8 1 2 3 4 5 6 ... 4 5 6 7 8 1 2 3 4 5 6 7 8""" - ) # noqa: E501 + - level_2 (x) int64 1 2 3 4 5 6 7 8 1 2 3 4 5 6 ... 4 5 6 7 8 1 2 3 4 5 6 7 8""" # noqa: E501 + ) assert expected == repr(mda_long) def test_properties(self): diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index d2c5c8e4905..43551d62265 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -101,7 +101,7 @@ def test_getitem_1d_fancy(self): ind = Variable(("a", "b"), [[0, 1], [0, 1]]) v_new = v[ind] assert v_new.dims == ("a", "b") - expected = np.array(v._data)[([0, 1], [0, 1]),] + expected = np.array(v._data)[([0, 1], [0, 1]),] # noqa assert_array_equal(v_new, expected) # boolean indexing