Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Black for code style and enforce in the CI #1132

Merged
merged 19 commits into from
Dec 23, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .flake8
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[flake8]
ignore = E501,E999,E741,E126,E127,F401,F403,F405,F811,F841,E743,W503
max-line-length = 100
ignore = E203,E999,E741,E126,E127,F401,F403,F405,F811,F841,E743,W503
exclude = gen3,external
9 changes: 9 additions & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ typecheck:
- make requirements
- make typecheck

formatcheck:
image: python:3.6
tags:
- github
script:
- make install
- make requirements
- make formatcheck

docs:
image: python:3.6
tags:
Expand Down
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ Changelog

### Improvements and Changes

- Some of the type hints have been added to the `Program` class (@rht, gh-1115)
- Added some type hints to the `Program` class (@rht, gh-1115).
- Use [Black](https://black.readthedocs.io/en/stable/index.html) for code style
and enforce it (along with a line length of 100) via the `style` (`flake8`)
and `formatcheck` (`black --check`) CI jobs (@karalekas, gh-1132).

### Bugfixes

Expand Down Expand Up @@ -42,7 +45,7 @@ Changelog
set to `LOG_LEVEL=DEBUG` to help diagnose problems. In addition, certain errors
will no longer print their entire stack trace outside of `DEBUG` mode, for a
cleaner console and better user experience. This is only true for errors where
the cause is well known. (@kalzoo, gh-1123)
the cause is well known (@kalzoo, gh-1123).
- Connection to the QPU compiler now supports both ZeroMQ and HTTP(S)
(@kalzoo, gh-1127).
- Bump quilc / qvm parent Docker images to v1.15.1 (@karalekas, gh-1128).
Expand Down
40 changes: 28 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,34 @@ Developer How-Tos

### Style Guidelines

We use `flake8` to automatically lint the code and enforce style requirements as part of the
CI pipeline. You can run these style tests yourself locally by running `flake8 pyquil` (or
`make style`) in the top-level directory of the repository. If you aren't presented with any
errors, then that means your code is good enough for the linter. In addition to these tests,
we have a collection of self-imposed style guidelines regarding typing, docstrings, and line
length:

1. Use type hints for parameters and return types with the
[PEP 484 syntax](https://www.python.org/dev/peps/pep-0484/).
2. Write useful [Sphinx-style](https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html)
docstrings, but without the `type` and `rtype` entries (use type hints instead).
3. Limit line length to 100 characters in code and documentation.
We use [Black](https://black.readthedocs.io/en/stable/index.html) and `flake8` to automatically
lint the code and enforce style requirements as part of the CI pipeline. You can run these style
tests yourself locally by running `make style` (to check for violations of the `flake8` rules)
and `make formatcheck` (to see if `black` would reformat the code) in the top-level directory of
the repository. If you aren't presented with any errors, then that means your code is good enough
for the linter (`flake8`) and formatter (`black`). If `make formatcheck` fails, it will present
you with a diff, which you can resolve by running `make format`. Black is very opinionated, but
saves a lot of time by removing the need for style nitpicks in PR review. We only deviate from its
default behavior in one category: we choose to use a line length of 100 rather than the Black
default of 88 (this is configured in the [`pyproject.toml`](pyproject.toml) file).

In addition to linting and formatting, we are in the process of rolling out the use of type hints
for all parameters and return values, using the [PEP 484 syntax][pep-484]. This is being done on
a file-by-file basis, and for ones that have been completed we now have a `make typecheck` command
that will enforce the use of types in those files as part of the CI. When a file is transitioned,
it should be added to the list in the `typecheck` target of the [`Makefile`](Makefile). Because we
use the `typing` module, types (e.g. `type` and `rtype` entries) should be omitted when writing
(useful) [Sphinx-style](https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html)
docstrings for classes, methods, and functions.

As useful shorthand, all of these style-related tests can be run locally with a single command,
by running the following:

```bash
make checkall
```

[pep-484]: https://www.python.org/dev/peps/pep-0484/

### Running the Unit Tests

Expand Down
11 changes: 11 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ DOCKER_TAG=rigetti/forest:$(COMMIT_HASH)
.PHONY: all
all: dist

.PHONY: checkall
checkall: formatcheck style typecheck

.PHONY: clean
clean:
rm -rf dist
Expand All @@ -32,6 +35,14 @@ docs: CHANGELOG.md
docker: Dockerfile
docker build -t $(DOCKER_TAG) .

.PHONY: format
format:
black pyquil

.PHONY: formatcheck
formatcheck:
black --check --diff pyquil

.PHONY: info
info:
python -V
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ PyQuil: Quantum programming in Python
[![docs status](https://readthedocs.org/projects/pyquil/badge/?version=latest)](http://pyquil.readthedocs.io/en/latest/?badge=latest)
[![pypi downloads](https://img.shields.io/pypi/dm/pyquil.svg)](https://pypi.org/project/pyquil/)
[![pypi version](https://img.shields.io/pypi/v/pyquil.svg)](https://pypi.org/project/pyquil/)
[![conda-forge version](https://img.shields.io/conda/vn/conda-forge/pyquil.svg)](https://anaconda.org/conda-forge/pyquil)
[![slack workspace](https://img.shields.io/badge/slack-rigetti--forest-812f82.svg?)][slack_invite]
[![code-style][black-badge]][black-repo]

[black-repo]: https://github.com/psf/black
[black-badge]: https://img.shields.io/badge/code%20style-black-000000.svg

PyQuil is a Python library for quantum programming using [Quil](https://arxiv.org/abs/1608.03355),
the quantum instruction language developed at [Rigetti Computing](https://www.rigetti.com/).
Expand Down
162 changes: 74 additions & 88 deletions conftest.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@

import shutil

import numpy as np
import pytest
from requests import RequestException

from pyquil.api import (QVMConnection, QVMCompiler, ForestConnection,
get_benchmarker, local_forest_runtime)
from pyquil.api import (
QVMConnection,
QVMCompiler,
ForestConnection,
get_benchmarker,
local_forest_runtime,
)
from pyquil.api._config import PyquilConfig
from pyquil.api._errors import UnknownApiError
from pyquil.api._compiler import QuilcNotRunning, QuilcVersionMismatch
Expand All @@ -20,43 +24,28 @@
@pytest.fixture
def isa_dict():
return {
"1Q": {
"0": {
'type': 'Xhalves'
},
"1": {},
"2": {},
"3": {
"dead": True
}
},
"1Q": {"0": {"type": "Xhalves"}, "1": {}, "2": {}, "3": {"dead": True}},
"2Q": {
"0-1": {},
"1-2": {
"type": "ISWAP"
},
"0-2": {
"type": "CPHASE"
},
"0-3": {
"dead": True
}
}
"1-2": {"type": "ISWAP"},
"0-2": {"type": "CPHASE"},
"0-3": {"dead": True},
},
}


@pytest.fixture
def specs_dict():
return {
'1Q': {
"1Q": {
"0": {
"f1QRB": 0.99,
"f1QRB_std_err": 0.01,
"f1Q_simultaneous_RB": 0.98,
"f1Q_simultaneous_RB_std_err": 0.02,
"fRO": 0.93,
"T1": 20e-6,
"T2": 15e-6
"T2": 15e-6,
},
"1": {
"f1QRB": 0.989,
Expand All @@ -65,7 +54,7 @@ def specs_dict():
"f1Q_simultaneous_RB_std_err": 0.021,
"fRO": 0.92,
"T1": 19e-6,
"T2": 12e-6
"T2": 12e-6,
},
"2": {
"f1QRB": 0.983,
Expand All @@ -74,7 +63,7 @@ def specs_dict():
"f1Q_simultaneous_RB_std_err": 0.027,
"fRO": 0.95,
"T1": 21e-6,
"T2": 16e-6
"T2": 16e-6,
},
"3": {
"f1QRB": 0.988,
Expand All @@ -83,70 +72,58 @@ def specs_dict():
"f1Q_simultaneous_RB_std_err": 0.022,
"fRO": 0.94,
"T1": 18e-6,
"T2": 11e-6
}
},
'2Q': {
"0-1": {
"fBellState": 0.90,
"fCZ": 0.89,
"fCZ_std_err": 0.01,
"fCPHASE": 0.88
"T2": 11e-6,
},
"1-2": {
"fBellState": 0.91,
"fCZ": 0.90,
"fCZ_std_err": 0.12,
"fCPHASE": 0.89
},
"0-2": {
"fBellState": 0.92,
"fCZ": 0.91,
"fCZ_std_err": 0.20,
"fCPHASE": 0.90
},
"0-3": {
"fBellState": 0.89,
"fCZ": 0.88,
"fCZ_std_err": 0.03,
"fCPHASE": 0.87
}
}
},
"2Q": {
"0-1": {"fBellState": 0.90, "fCZ": 0.89, "fCZ_std_err": 0.01, "fCPHASE": 0.88},
"1-2": {"fBellState": 0.91, "fCZ": 0.90, "fCZ_std_err": 0.12, "fCPHASE": 0.89},
"0-2": {"fBellState": 0.92, "fCZ": 0.91, "fCZ_std_err": 0.20, "fCPHASE": 0.90},
"0-3": {"fBellState": 0.89, "fCZ": 0.88, "fCZ_std_err": 0.03, "fCPHASE": 0.87},
},
}


@pytest.fixture
def noise_model_dict():
return {'gates': [{'gate': 'I',
'params': (5.0,),
'targets': (0, 1),
'kraus_ops': [[[[1.]], [[1.0]]]],
'fidelity': 1.0},
{'gate': 'RX',
'params': (np.pi / 2.,),
'targets': (0,),
'kraus_ops': [[[[1.]], [[1.0]]]],
'fidelity': 1.0}],
'assignment_probs': {'1': [[1.0, 0.0], [0.0, 1.0]],
'0': [[1.0, 0.0], [0.0, 1.0]]},
}
return {
"gates": [
{
"gate": "I",
"params": (5.0,),
"targets": (0, 1),
"kraus_ops": [[[[1.0]], [[1.0]]]],
"fidelity": 1.0,
},
{
"gate": "RX",
"params": (np.pi / 2.0,),
"targets": (0,),
"kraus_ops": [[[[1.0]], [[1.0]]]],
"fidelity": 1.0,
},
],
"assignment_probs": {"1": [[1.0, 0.0], [0.0, 1.0]], "0": [[1.0, 0.0], [0.0, 1.0]]},
}


@pytest.fixture
def device_raw(isa_dict, noise_model_dict, specs_dict):
return {'isa': isa_dict,
'noise_model': noise_model_dict,
'specs': specs_dict,
'is_online': True,
'is_retuning': False}
return {
"isa": isa_dict,
"noise_model": noise_model_dict,
"specs": specs_dict,
"is_online": True,
"is_retuning": False,
}


@pytest.fixture
def test_device(device_raw):
return Device('test_device', device_raw)
return Device("test_device", device_raw)


@pytest.fixture(scope='session')
@pytest.fixture(scope="session")
def qvm():
try:
qvm = QVMConnection(random_seed=52)
Expand All @@ -171,7 +148,7 @@ def compiler(test_device):
return pytest.skip("This test requires a different version of quilc: {}".format(e))


@pytest.fixture(scope='session')
@pytest.fixture(scope="session")
def forest():
try:
connection = ForestConnection()
Expand All @@ -181,42 +158,51 @@ def forest():
return pytest.skip("This test requires a Forest connection: {}".format(e))


@pytest.fixture(scope='session')
@pytest.fixture(scope="session")
def benchmarker():
try:
bm = get_benchmarker(timeout=2)
bm.apply_clifford_to_pauli(Program(I(0)), sX(0))
return bm
except (RequestException, TimeoutError) as e:
return pytest.skip("This test requires a running local benchmarker endpoint (ie quilc): {}"
.format(e))
return pytest.skip(
"This test requires a running local benchmarker endpoint (ie quilc): {}".format(e)
)


@pytest.fixture(scope='session')
@pytest.fixture(scope="session")
def local_qvm_quilc():
"""Execute test with local qvm and quilc running"""
if shutil.which('qvm') is None or shutil.which('quilc') is None:
return pytest.skip("This test requires 'qvm' and 'quilc' "
"executables to be installed locally.")
if shutil.which("qvm") is None or shutil.which("quilc") is None:
return pytest.skip(
"This test requires 'qvm' and 'quilc' " "executables to be installed locally."
karalekas marked this conversation as resolved.
Show resolved Hide resolved
)

with local_forest_runtime() as context:
yield context


def _str_to_bool(s):
"""Convert either of the strings 'True' or 'False' to their Boolean equivalent"""
if s == 'True':
if s == "True":
return True
elif s == 'False':
elif s == "False":
return False
else:
raise ValueError("Please specify either True or False")


def pytest_addoption(parser):
parser.addoption("--use-seed", action="store", type=_str_to_bool, default=True,
help="run operator estimation tests faster by using a fixed random seed")
parser.addoption("--runslow", action="store_true", default=False, help="run tests marked as being 'slow'")
parser.addoption(
"--use-seed",
action="store",
type=_str_to_bool,
default=True,
help="run operator estimation tests faster by using a fixed random seed",
)
parser.addoption(
"--runslow", action="store_true", default=False, help="run tests marked as being 'slow'"
)


def pytest_configure(config):
Expand Down
Loading