Type annotations for gates, quilatom, and quilbase modules

[appleby]

Description

This PR adds type annotations for the pyquil.gates, pyquil.quilatom, and pyquil.quilbase modules.

This is still a WIP, but is ready for review. Since I'm new-ish to pyQuil, I probably got the types wrong in several places. See the TODOs in the diff for places I'm particularly unsure about. Any hints from reviewers on these are greatly appreciated.

So far, these pass muster when running mypy like so:

mypy --ignore-missing-imports --follow-imports=silent pyquil/quilatom.py
mypy --ignore-missing-imports --follow-imports=silent pyquil/quilbase.py
mypy --ignore-missing-imports --follow-imports=silent pyquil/gates.py

Resolves #988

TODO:

• Address remaining TODO:(appleby)s in the source
• Check rigetti/quil, ensure types agree with spec
• Turn on mypy's --strict flag and fix any errors reported
• Run mypy on full source tree, attempt to pick out the relevant errors drowning in the sea of breakage
• Update docstring types
• Update the changelog

Checklist

• The above description motivates these changes.
• There is a unit test that covers these changes.
• All new and existing tests pass locally and on Semaphore.
• Parameters have type hints with PEP 484 syntax.
• Functions and classes have useful sphinx-style docstrings.
• (New Feature) The docs have been updated accordingly.
• (Bugfix) The associated issue is referenced above using
  auto-close keywords.
• The changelog is updated,
  including author and PR number (@username, gh-xxx).

[appleby]

The classical_reg arg here can also be an int, but that is deprecated. The current type hint causes mypy to warn:

pyquil/tests/test_qvm.py:16: error: Argument 2 to "MEASURE" has incompatible type "int"; expected "Union[MemoryReference, Tuple[str, int], List[Any], str, None]"

This warning only pops up in the three places in test_qvm.py. We could update the type hint here to include the deprecated int type and get rid of the warnings, but it seems saner to leave it as advertising only non-deprecated types.

[appleby]

This is ready for review. There is still a remaining TODO to update the docstrings, but I'd rather go through a round of review first to make sure the types look sane, then update the docstrings once at the end.

[notmgsk]

Small nits for your picking.

[notmgsk]

Can't be the most exciting thing you've worked on! :D

[appleby]

Can't be the most exciting thing you've worked on! :D

It's a dirty job, but somebody gotta do it.

[karalekas]

@appleby is there some way we can get around the init -> None thing? seems like Guido himself is not a fan python/mypy#604 (comment)

[appleby]

@appleby is there some way we can get around the init -> None thing? seems like Guido himself is not a fan python/mypy#604 (comment)

I agree with Guido here, but sadly mypy seems to require them. I think we can drop the -> None from any __init__ methods that have at least one non-self argument which is typed. But as far as I understand it, the explicit -> None return type specifier is still required by mypy for __init__s with no annotated args when called from a type-checked context.

On the other hand, I think mypy only sees this as an error when run in --strict mode, so if we're ok with ignoring those for now, then I could nix them. Once we get around to adding to type hints for the quilbase module, practically all of the constructors there take arguments and therefore wouldn't needed an explicit return type annotation.

[ecpeterson]

I have not read this PR line-by-line, but I approve of its spirit.

[appleby]

Here is a list of the new type aliases added in this PR:

pyquil/api/_quantum_computer.py
ExecutableDesignator = Union[BinaryExecutableResponse, PyQuilExecutableResponse]

pyquil/quilatom.py
QubitDesignator = Union[Qubit, QubitPlaceholder, int]
MemoryReferenceDesignator = Union['MemoryReference', Tuple[str, int], List[Any], str]
ParameterDesignator = Union['Expression', 'MemoryReference', np.int_, int, float, complex]
ExpressionValueDesignator = Union[int, float, complex]
ExpressionDesignator = Union['Expression', ExpressionValueDesignator]
ParamSubstitutionsMapDesignator = Mapping['Parameter', ExpressionValueDesignator]

A brief explanation for some of the less obvious types:

MemoryReferenceDesignator is used anywhere a memory reference is expected. This means the function in question accepts any of the listed types and hands the value off to unpack_classical_reg to turn it into a valid MemoryReference.

ExpressionValueDesignator captures the set of possible types for an evaluated Quil expression like i*sin(pi), e.g.

ParameterDesignator is used for gate parameters. There is an open question of whether this type hint should include complex or not. See e.g. #1009 and quil-lang/quilc#313.

ParamSubstitutionsMapDesignator is used in the _substitute methods of the various Expression subclasses to indicate the type of the substitutions dict argument, which maps a Parameter to the ExpressionValue that will replace it.

edit: updated with new type aliases from latest commit

[appleby]

Last TODO of updating dosctrings is now done. Marking this ready for review.

I thought I was going to have to add explicit types to the docstring of every function that I added type hints for, hence why I was procrastinating on that thankless task. Finally started doing it, got annoyed and thought: "surely there is a sphinx plugin to figure this out from the type hints in the source", googled it and realized we are already using sphinx-autodoc-typehints (yay). So I ended up removing a handful of explicit references to argument and return types in the docstrings in order to allow sphinx-autodoc-typehints to take care of it and prevent the docstrings from going stale.

[karalekas]

looks great!