Skip to content

Commit

Permalink
Make it clearer dev-requirements does not contain dependencies by mov…
Browse files Browse the repository at this point in the history
…ing it to dev_tools/conf/pip-list-dev-tools.txt (#804)

- Rename runtime-requirements.txt to just requirements.txt
- Inline contrib-requirements.txt into requirements.txt
- Refactor create_virtual_env to take multiple requirements files
- Move configuration files (e.g. .pylintrc) into dev_tools/conf
- Rename apt-runtime-requirements to apt-system-requirements
- Expand development documentation to discuss tools in check/ and expand on how to create a package
  • Loading branch information
Strilanc authored Aug 8, 2018
1 parent c930960 commit 9841cfc
Show file tree
Hide file tree
Showing 27 changed files with 114 additions and 67 deletions.
10 changes: 6 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,28 @@ matrix:
env: NAME=mypy
python: "3.5"
install:
- cat dev-requirements.txt | grep mypy | xargs pip install
- cat dev_tools/conf/pip-list-dev-tools.txt | grep mypy | xargs pip install
script: check/mypy

- os: linux
env: NAME=pylint
python: "3.5"
install:
- cat dev-requirements.txt | grep pylint | xargs pip install
- cat dev_tools/conf/pip-list-dev-tools.txt | grep pylint | xargs pip install
script: check/pylint

- os: linux
env: NAME=pytest-and-incremental-coverage
python: "3.5"
install:
- pip install -r dev-requirements.txt
- pip install -r requirements.txt
- pip install -r dev_tools/conf/pip-list-dev-tools.txt
script: check/pytest-and-incremental-coverage master

- os: linux
env: NAME=pytest2
python: "2.7"
install:
- pip install -r python2.7-dev-requirements.txt
- pip install -r python2.7-requirements.txt
- pip install -r dev_tools/conf/pip-list-python2.7-test-tools.txt
script: continuous-integration/travis-pytest2.sh
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ To ignore coverage for an entire block, start the block with a `# coverage: igno
- **Lint**.
Code should meet common style standards for python and be free of error-prone constructs.
We use [pylint](https://www.pylint.org/) to check for lint.
To see which lint checks we enforce, see the [continuous-integration/.pylintrc](continuous-integration/.pylintrc) file.
To see which lint checks we enforce, see the [dev_tools/conf/.pylintrc](dev_tools/conf/.pylintrc) file.
When pylint produces a false positive, it can be squashed with annotations like `# pylint: disable=unused-import`.
- **Types**.
Code should have [type annotations](https://www.python.org/dev/peps/pep-0484/).
Expand Down
2 changes: 1 addition & 1 deletion MANIFEST.in
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
include runtime-requirements.txt
include requirements.txt
include LICENSE
File renamed without changes.
2 changes: 1 addition & 1 deletion check/mypy
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
cd "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
cd $(git rev-parse --show-toplevel)

mypy --config-file=continuous-integration/mypy.ini $@ .
mypy --config-file=dev_tools/conf/mypy.ini $@ .
2 changes: 1 addition & 1 deletion check/pylint
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
cd "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
cd $(git rev-parse --show-toplevel)

pylint --rcfile=continuous-integration/.pylintrc $@ cirq dev_tools examples
pylint --rcfile=dev_tools/conf/.pylintrc $@ cirq dev_tools examples
4 changes: 3 additions & 1 deletion cirq/contrib/contrib-requirements.txt
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
pylatex~=1.3
# Runtime requirements for the python 3 version of cirq.

pylatex==1.3.*
3 changes: 1 addition & 2 deletions continuous-integration/build-docs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,5 @@ if [ -d "$repo_root/docs/generated" ] ; then
fi

cd "$repo_root/docs"
pip install -r dev-requirements.txt
pip install -r requirements.txt
make html

9 changes: 0 additions & 9 deletions dev-requirements.txt

This file was deleted.

1 change: 0 additions & 1 deletion dev_tools/bash_scripts_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ def test_pytest_changed_files_branch_selection():
"Found 0 differing files with associated tests.\n")



@only_in_python3_on_posix
def test_pytest_and_incremental_coverage_branch_selection():

Expand Down
3 changes: 2 additions & 1 deletion dev_tools/check_pylint.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ def context(self):
def perform_check(self, env: env_tools.PreparedEnv, verbose: bool):
base_path = cast(str, env.destination_directory)
rc_path = os.path.join(base_path,
'continuous-integration',
'dev_tools',
'conf',
'.pylintrc')
files = list(
env_tools.get_unhidden_ungenerated_python_files(base_path))
Expand Down
3 changes: 2 additions & 1 deletion dev_tools/check_pytest_with_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ def perform_check(self, env: env_tools.PreparedEnv, verbose: bool):
do_coverage = True
base_path = cast(str, env.destination_directory)
rc_path = os.path.join(base_path,
'continuous-integration',
'dev_tools',
'conf',
'.coveragerc')
target_path = base_path
result = shell_tools.run_cmd(
Expand Down
3 changes: 2 additions & 1 deletion dev_tools/check_typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def context(self):
def perform_check(self, env: env_tools.PreparedEnv, verbose: bool):
base_path = cast(str, env.destination_directory)
config_path = os.path.join(base_path,
'continuous-integration',
'dev_tools',
'conf',
'mypy.ini')
files = list(env_tools.get_unhidden_ungenerated_python_files(
base_path))
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
6 changes: 6 additions & 0 deletions dev_tools/conf/pip-list-dev-tools.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
3to2==1.*
absl-py==0.2.*
mypy==0.610.*
pylint==1.9.*
pytest==3.6.*
pytest-cov==2.5.*
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
-r python2.7-runtime-requirements.txt

3to2~=1.1
absl-py~=0.2
mock~=2.0
Expand Down
31 changes: 19 additions & 12 deletions dev_tools/env_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ def get_unhidden_ungenerated_python_files(directory: str) -> Iterable[str]:


def create_virtual_env(venv_path: str,
requirements_path: str,
requirements_paths: Iterable[str],
python_path: str,
verbose: bool) -> None:
"""Creates a new virtual environment and then installs dependencies.
Args:
venv_path: Where to put the virtual environment's state.
requirements_path: Location of the requirements file to -r install.
requirements_paths: Location of requirements files to -r install.
python_path: The python binary to use.
verbose: When set, more progress output is produced.
"""
Expand All @@ -60,12 +60,13 @@ def create_virtual_env(venv_path: str,
venv_path,
out=sys.stderr)
pip_path = os.path.join(venv_path, 'bin', 'pip')
shell_tools.run_cmd(pip_path,
'install',
None if verbose else '--quiet',
'-r',
requirements_path,
out=sys.stderr)
for req_path in requirements_paths:
shell_tools.run_cmd(pip_path,
'install',
None if verbose else '--quiet',
'-r',
req_path,
out=sys.stderr)


def prepare_temporary_test_environment(
Expand Down Expand Up @@ -115,10 +116,14 @@ def prepare_temporary_test_environment(
# Create virtual environment.
base_path = cast(str, env.destination_directory)
env_path = os.path.join(base_path, env_name)
req_path = os.path.join(base_path, 'dev-requirements.txt')
req_path = os.path.join(base_path, 'requirements.txt')
req_path_2 = os.path.join(base_path,
'dev_tools',
'conf',
'pip-list-dev-tools.txt')
create_virtual_env(venv_path=env_path,
python_path=python_path,
requirements_path=req_path,
requirements_paths=[req_path, req_path_2],
verbose=verbose)

return PreparedEnv(github_repo=env.repository,
Expand Down Expand Up @@ -163,10 +168,12 @@ def derive_temporary_python2_environment(

# Create virtual environment.
env_path = os.path.join(destination_directory, env_name)
req_path = os.path.join(destination_directory, 'dev-requirements.txt')
# (These files are output by python2.7-generate.sh.)
req_path = os.path.join(destination_directory, 'requirements.txt')
req_path_2 = os.path.join(destination_directory, 'pip-list-test-tools.txt')
create_virtual_env(venv_path=env_path,
python_path=python_path,
requirements_path=req_path,
requirements_paths=[req_path, req_path_2],
verbose=verbose)

return PreparedEnv(github_repo=python3_environment.repository,
Expand Down
7 changes: 1 addition & 6 deletions dev_tools/run_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import sys
import tempfile

from dev_tools import env_tools, shell_tools, all_checks, check
from dev_tools import env_tools, all_checks, check


REPO_ORGANIZATION = 'quantumlib'
Expand Down Expand Up @@ -82,11 +82,6 @@ def parse_args():

def main():
pull_request_number, access_token, verbose, checks = parse_args()
if pull_request_number is None:
print(shell_tools.highlight(
'No pull request number given. Using local files.',
shell_tools.YELLOW))
print()

test_dir = tempfile.mkdtemp(prefix='test-{}-'.format(REPO_NAME))
test_dir_2 = tempfile.mkdtemp(prefix='test-{}-py2-'.format(REPO_NAME))
Expand Down
76 changes: 59 additions & 17 deletions docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ See the previous section for instructions.
You can install most other dependencies via `apt-get`:

```bash
cat apt-dev-requirements.txt apt-runtime-requirements.txt | xargs sudo apt-get install --yes
cat apt-system-requirements.txt dev_tools/conf/apt-list-dev-tools.txt | xargs sudo apt-get install --yes
```

Unfortunately, as of this writing, v3.5 of the [protobuf compiler](https://github.com/google/protobuf) is not installable via `apt-get`.
Expand All @@ -89,15 +89,16 @@ See the previous section for instructions.
If you want to be able to run the python 2.7 tests on your machine, see ["Protobuf Compiler Installation" on the google/protobuf github repository](https://github.com/google/protobuf#protocol-compiler-installation) for details.


2. Prepare a virtual environment with the dev requirements.
2. Prepare a virtual environment including the dev tools (such as mypy).

One of the system dependencies we installed was `virtualenvwrapper`, which makes it easy to create virtual environment.
If you did not have `virtualenvwrapper` previously, you may need to re-open your terminal or run `source ~/.bashrc` before these commands will work:

```bash
mkvirtualenv cirq-py3 --python=/usr/bin/python3
pip install --upgrade pip
pip install -r dev-requirements.txt
pip install -r requirements.txt
pip install -r dev_tools/conf/pip-list-dev-tools.txt
```

(When you later open another terminal, you can activate the virtualenv with `workon cirq-py3`.)
Expand All @@ -119,34 +120,51 @@ See the previous section for instructions.

There are a few options for running continuous integration checks, varying from easy and fast to slow and reliable.

The simplest and fastest way to run checks is to invoke `pytest`, `pylint`, or `mypy` for yourself as follows:
The simplest way to run checks is to invoke `pytest`, `pylint`, or `mypy` for yourself as follows:

```bash
pytest .
pylint --rcfile=continuous-integration/.pylintrc cirq # note: only tests the cirq subdirectory
mypy --config-file=continuous-integration/mypy.ini .
pytest
pylint --rcfile=dev_tools/conf/.pylintrc cirq
mypy --config-file=dev_tools/conf/mypy.ini .
```

But note that this will not check the incremental coverage or run the python 2 tests, and that during continuous integration pylint is run against more than just the cirq subdirectory.

The script [continuous-integration/simple_check.sh](/continuous-integration/simple_check.sh) will run pytest, mypy, pylint (on all the necessary files) and the incremental coverage check:
This can be a bit tedious, because you have to specify the configuration files each time.
A more convenient way to run checks is to via the scripts in the [check/](/check) directory, which specify configuration arguments for you and cover more use cases:

```bash
bash continuous-integration/simple_check.sh
```
# Run all tests in the repository.
./check/pytest
# Check all relevant files in the repository for lint.
./check/pylint
# Typecheck all python files in the repository.
./check/mypy
# Transpile to python 2 and run tests.
./check/pytest2 # Note: must be in a python 2 virtual env to run this.
But this script doesn't guarantee your test environment is up to date, and it does not transpile and test the python 2 code.
# Compute incremental coverage vs master (or a custom revision of your choice).
./check/pytest-and-incremental-coverage [BASE_REVISION]
Use the script [continuous-integration/check.sh](/continuous-integration/check.sh) to do a proper full local check.
This script will create (temporary) virtual environments, do a fresh install of all relevant dependencies, transpile the python 2 code, and run all relevant checks.
# Only run tests associated with files that have changed when diffed vs master (or a custom revision of your choice).
./check/pytest-changed-files [BASE_REVISION]
```

The above scripts are convenient and reasonably fast, but they often don't match the results computed by the continuous integration builds run on travis.
For example, you may be running an older version of `pylint` or `numpy`.
In order to run a check that is significantly more likely to agree with the travis builds, you can use the [continuous-integration/check.sh](/continuous-integration/check.sh) script:
```bash
bash continuous-integration/check.sh
./continuous-integration/check.sh
```
You can run a subset of the checks using the ```--only``` flag.
This script will create (temporary) virtual environments, do a fresh install of all relevant dependencies, transpile the python 2 code, and run all relevant checks within those clean environments.
Note that creating the virtual environments takes time, and prevents some caching mechanisms from working, so `continuous-integration/check.sh` is significantly slower than the simpler check scripts.
When using this script, you can run a subset of the checks using the ```--only``` flag.
This flag value can be `pylint`, `typecheck`, `pytest`, `pytest2`, or `incremental-coverage`.
### Producing the Python 2.7 code
Run [python2.7-generate.sh](/python2.7-generate.sh) to transpile cirq's python 3 code into python 2.7 code:
Expand All @@ -171,10 +189,34 @@ The script does nothing if the output directory already exists.
`pip` will choose between the two based on whichever version of python the user is using.
Development versions end with `.dev35` and `.dev27` instead of `.35` and `.27`, e.g. use `0.0.4.dev27` for the python 2 variant of the development version of `0.0.4`.
Create a pull request turning `0.0.X.*dev` into `0.0.X.*`, and a followup pull request turning `0.0.X.*` into `0.0.X+1.*dev`.
2. Run [dev_tools/prepare-package.sh](/dev_tools/produce-package.sh) to produce pypi artifacts.
```bash
bash dev_tools/produce-package.sh
```
The output files will be placed in `dist/`.
3. Do a quick test run of the packages.
Create fresh python 3 and python 2 virtual environments, and try to `pip install` the produced artifacts.
Check that `import cirq` actually finishes after installing.
The output files will be placed in `dist/`, from which they can be uploaded to pypi with a tool such as `twine`.
4. Create a github release.
Describe major changes (especially breaking changes) in the summary.
Make sure you point the tag being created at the one and only revision with the non-dev version number.
Attach the package files you produced to the release.
5. Upload to pypi.
You can use a tool such as `twine` for this.
For example:
```bash
twine upload -u "${PYPI_USERNAME}" -p "${PYPI_PASSWORD}" dist/*
```
5 changes: 2 additions & 3 deletions python2.7-generate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ find ${proto_dir} | grep '_pb2\.py' | xargs rm -f
protoc -I="${out_dir}" --python_out="${out_dir}" ${proto_dir}/*.proto

# Include requirements files.
cp "${in_dir}/python2.7-runtime-requirements.txt" "${out_dir}/runtime-requirements.txt"
cp "${in_dir}/python2.7-dev-requirements.txt" "${out_dir}/dev-requirements.txt"
sed -i -e 's/python2.7-runtime-requirements.txt/runtime-requirements.txt/g' "${out_dir}/dev-requirements.txt"
cp "${in_dir}/python2.7-requirements.txt" "${out_dir}/requirements.txt"
cp "${in_dir}/dev_tools/conf/pip-list-python2.7-test-tools.txt" "${out_dir}/pip-list-test-tools.txt"

# Include packaging files.
cp "${in_dir}/MANIFEST.in" "${out_dir}/MANIFEST.in"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Runtime requirements for the transpiled python 2.7 version of cirq.

google-api-python-client~=1.6
matplotlib~=2.1
networkx~=2.1
Expand Down
4 changes: 3 additions & 1 deletion runtime-requirements.txt → requirements.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# Runtime requirements for the python 3 version of cirq.

google-api-python-client~=1.6
matplotlib~=2.2
networkx~=2.1
numpy~=1.14
protobuf~=3.5
requests~=2.18
sortedcontainers~=2.0
scipy~=1.1.0
scipy
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
long_description += stream.read()

# Read in requirements
requirements = open('runtime-requirements.txt').readlines()
requirements = open('requirements.txt').readlines()
requirements = [r.strip() for r in requirements]

cirq_packages = ['cirq'] + [
Expand Down
2 changes: 1 addition & 1 deletion style.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
As mentioned in [CONTRIBUTING.md](CONTRIBUTING.md) we use use [pylint](https://www.pylint.org/)
to check for style violations. Pylint attempts to enforce styles in
[PEP 8](https://www.python.org/dev/peps/pep-0008/). To see which lint checks we enforce, see the
[continuous-integration/.pylintrc](continuous-integration/.pylintrc) file.
[dev_tools/conf/.pylintrc](dev_tools/conf/.pylintrc) file.

Here we include some extra style guidelines.

Expand Down

0 comments on commit 9841cfc

Please sign in to comment.