Skip to content
This repository has been archived by the owner on Jul 8, 2021. It is now read-only.

Blackify #14

Merged
merged 19 commits into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
# https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes

ignore =
# E203: whitespace before ':'
E203,
# E402: module level imports at top of file
E402,
trexfeathers marked this conversation as resolved.
Show resolved Hide resolved
# E501: line too long
E501,
# W503: line break before binary operator
W503,
# W504: line break after binary operator
Expand Down
27 changes: 27 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: 'v3.2.0'
hooks:
# Prevent giant files from being committed.
- id: check-added-large-files
# Check for files that contain merge conflict strings.
- id: check-merge-conflict
# Check for debugger imports and py37+ `breakpoint()` calls in python source.
- id: debug-statements
# Don't commit to master branch.
- id: no-commit-to-branch
- repo: https://github.com/psf/black
rev: '19.10b0'
hooks:
- id: black
# Force black to run on whole repo, using settings from pyproject.toml
pass_filenames: false
args: [--config=./pyproject.toml, .]
- repo: https://gitlab.com/pycqa/flake8
rev: '3.8.3'
hooks:
# Run flake8.
- id: flake8
args: [--config=./.flake8]
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ install:
conda install --quiet -n ${ENV_NAME} --file ${CONDA_REQS_FILE};

# Conda-install our own additional dependencies.
- conda install --file conda_requirements.txt
- conda install --file requirements/conda_requirements.txt

# Output environment debug info
- >
Expand Down Expand Up @@ -90,4 +90,4 @@ install:

script:

- pytest -v ./iris_ugrid/tests
- pytest -v --black ./iris_ugrid/tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems different to the way Iris is set up to test for black compliance
https://github.com/SciTools/iris/blob/d16f6767d4d775e2afd691de9c6514782ba2a90d/.travis.yml#L108-L113
In particular, it looks as though black --check is being run for iris. Is this test equivalent? If there is a difference, is there a reason for that difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping to leverage the extra elegance of pytest (which Iris doesn't use) by using the pytest-black plugin. Unfortunately, while pytest-black is under active development, it is not kept up to date on Anaconda, which has resulted in the deprecation error in Travis.

I am unsure whether the correct action should be to explicitly pip install pytest-black, or to invoke black the 'old-fashioned' way like Iris does. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... just trying the pip solution on Travis as a POC ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like the idea of using the latest features of pytest, since we are kind of using this repo to "test drive" pytest anyway. I guess there is a good case for making changes where pytest allows a more elegant solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK well that worked at least, and I've included a TODO to get rid of the pip line once conda gets a later pytest-black version.
Gonna take a break now. I await your verdict!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth noting that in Iris, black is pinned to version 19.10b0 https://github.com/SciTools/iris/blob/d16f6767d4d775e2afd691de9c6514782ba2a90d/requirements/test.txt#L4.
This approach means that Iris-ugrid will be tested against a different version of black than Iris. I don't think this should cause massive issues, but it might be worth thinking about.

7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,9 @@
<p align="center">
<!-- https://shields.io/ is a good source of these -->
<a href="https://github.com/psf/black">
<img src="https://img.shields.io/badge/code%20style-black-000000.svg"
alt="black" /></a>
</p>

# iris-ugrid
Unstructured mesh library for Iris
2 changes: 0 additions & 2 deletions conda_requirements.txt

This file was deleted.

5 changes: 2 additions & 3 deletions iris_ugrid/tests/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
def test_iris_installation():
# Check that iris cf loader includes 'exclude' functionality.
# Import a ugrid-specific test, that ought to exist on the branch.
from iris.tests.unit.fileformats.cf.test_CFReader \
import Test_exclude_vars
from iris.tests.unit.fileformats.cf.test_CFReader import Test_exclude_vars

assert hasattr(Test_exclude_vars, 'test_exclude_vars')
assert hasattr(Test_exclude_vars, "test_exclude_vars")
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tool.black]
line-length = 79
3 changes: 3 additions & 0 deletions requirements/conda_requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
black
gridded
pytest-black