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

Add CI checks for mypy and pylint #55

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
19 changes: 19 additions & 0 deletions .github/workflows/mypy.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: Type Check

on: [push, pull_request]

jobs:
mypy:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v1
with:
python-version: '3.8'
architecture: 'x64'
- name: Install mypy
run: |
python -m pip install --upgrade pip
pip install mypy
- name: Type check
run: dev_tools/mypy || true
19 changes: 19 additions & 0 deletions .github/workflows/pylint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: Pylint

on: [push, pull_request]

jobs:
pylint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v1
with:
python-version: '3.8'
architecture: 'x64'
- name: Install Pylint
run: |
python -m pip install --upgrade pip
pip install pylint
- name: Pylint check
run: dev_tools/pylint
73 changes: 73 additions & 0 deletions dev_tools/.pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
[MASTER]
load-plugins=pylint.extensions.docstyle,pylint.extensions.docparams,pylint_copyright_checker
max-line-length=100
Copy link
Collaborator

Choose a reason for hiding this comment

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

88 to agree with black?

disable=all
#ignore-paths=cirq-google/cirq_google/cloud/.*
ignore-patterns=.*_pb2\.py
output-format=colorized
score=no
reports=no
enable=
anomalous-backslash-in-string,
assert-on-tuple,
bad-indentation,
bad-option-value,
bad-reversed-sequence,
bad-super-call,
consider-merging-isinstance,
continue-in-finally,
dangerous-default-value,
docstyle,
duplicate-argument-name,
expression-not-assigned,
function-redefined,
inconsistent-mro,
init-is-generator,
line-too-long,
lost-exception,
missing-kwoa,
missing-param-doc,
missing-raises-doc,
mixed-line-endings,
no-value-for-parameter,
nonexistent-operator,
not-in-loop,
pointless-statement,
redefined-builtin,
return-arg-in-generator,
return-in-init,
return-outside-function,
simplifiable-if-statement,
singleton-comparison,
syntax-error,
too-many-function-args,
trailing-whitespace,
undefined-variable,
unexpected-keyword-arg,
unhashable-dict-key,
unnecessary-pass,
unreachable,
unrecognized-inline-option,
unused-import,
unnecessary-semicolon,
unused-variable,
unused-wildcard-import,
wildcard-import,
wrong-import-order,
wrong-import-position,
yield-outside-function

# Ignore long lines containing urls or pylint directives.
ignore-long-lines=^(.*#\w*pylint: disable.*|\s*(# )?[<\[\(]?https?://\S+[>\]\)]?)$

[TYPECHECK]

# List of members which are set dynamically and missed by pylint inference
# system, and so shouldn't trigger E1101 when accessed. Python regular
# expressions are accepted.
generated-members=numpy.*


#[IMPORTS]
# Force import order to recognize a module as part of a third party library.
#known-third-party=cirq,cirq_google,cirq_aqt,cirq_ionq
20 changes: 20 additions & 0 deletions dev_tools/mypy
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

################################################################################
# Runs mypy on the repository using a preconfigured mypy.ini file.
#
# Usage:
# check/mypy [--flags]
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"


echo -e -n "\033[31m"
mypy --config-file=dev_tools/mypy.ini "$@" .
result=$?
echo -e -n "\033[0m"

exit ${result}
27 changes: 27 additions & 0 deletions dev_tools/mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
[mypy]
show_error_codes = true

[mypy-__main__]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this mean?

follow_imports = silent
ignore_missing_imports = true

# 3rd-party libs for which we don't have stubs
[mypy-apiclient.*,freezegun.*,matplotlib.*,mpl_toolkits,multiprocessing.dummy,oauth2client.*,pandas.*,proto.*,pytest.*,scipy.*,sortedcontainers.*,setuptools.*,pylatex.*,networkx.*,qiskit.*,pypandoc.*,ply.*,_pytest.*,google.api.*,google.api_core.*,grpc.*,google.auth.*,google.oauth2.*,google.protobuf.text_format.*,quimb.*,pyquil.*,google.cloud.*,filelock.*,codeowners.*,tqdm.*,importlib_metadata.*,google.colab.*,IPython.*,astroid.*,pylint.*,cirq.*,flask.*,numpy.*,cirq_google.*]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow, do you really have to maintain a list of transitive dependencies to use mypy? I think this will be a problem (if the check is enforced). The set of dependencies can change at any time since the package manager generally downloads the latest versions modulo constraints. So this will lead to CI errors on pull requests unrelated to the actual changes whenever a new transitive dependency appears.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not transitive dependencies, it's only direct dependencies. CI errors for this will only be if a new dependency appears that was introduced in the actual PR.

Hopefully I can figure out how to get stubs for some of the libraries as well.
Let me see if I can clean this up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you mean it's not transitive dependencies? For example freezegun, protobuf, or flask are not referenced by the Unitary codebase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Those were all from cirq, where I copied the continuous integration test from. So, there were a lot of extra unneeded dependencies in there. If you refresh to the latest commit, I have pruned it way down (there may still be a few extra unneeded ones, but I got rid of a bunch of them).

follow_imports = silent
ignore_missing_imports = true

# There was no type information before numpy 1.20, so there are numpy mypy issues in the codebase
[mypy-numpy.*]
follow_imports = skip
follow_imports_for_stubs = true

# Treat symbols imported from Google's protobuf library as type Any.
# This supresses errors due to attributes not known to typeshed,
# e.g. Descriptor._options.
[mypy-google.protobuf.*]
follow_imports = skip
follow_imports_for_stubs = true

# ruamel is a downstream dependency of cirq-rigetti through pyquil.
[mypy-ruamel.*]
ignore_missing_imports = true
15 changes: 15 additions & 0 deletions dev_tools/pylint
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash

################################################################################
# Runs pylint on the repository using a preconfigured .pylintrc file.
#
# Usage:
# check/pylint [--flags for pylint]
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"

# Add dev_tools to $PYTHONPATH so that pylint can find custom checkers
env PYTHONPATH=dev_tools pylint --jobs=0 --rcfile=dev_tools/.pylintrc "$@" .
7 changes: 7 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,12 @@ cirq-google>=0.14.0

seaborn
sphinx

# dev tools
ipython
black
mypy
pylint
pytest
pytest-cov
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why install coverage tools?

coverage<=6.2