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

Reduce mypy errors in paulis.py #1147

Merged
merged 4 commits into from
Jan 6, 2020
Merged

Reduce mypy errors in paulis.py #1147

merged 4 commits into from
Jan 6, 2020

Conversation

rht
Copy link
Contributor

@rht rht commented Jan 1, 2020

Description

This currently reduces the mypy error from 31 down to 20. For the rest, I am not so sure how to handle them.

Checklist

  • The above description motivates these changes.
  • All new and existing tests pass locally and on Travis CI.
  • Parameters and return values have type hints with PEP 484 syntax.
  • Functions and classes have useful Sphinx-style docstrings.
  • All code follows Black style and obeys flake8 conventions.
  • The changelog is updated, including author and PR number (@username, gh-xxx).

@rht rht requested a review from a team as a code owner January 1, 2020 19:43
@karalekas karalekas added the quality 🎨 Improve code quality. label Jan 2, 2020
Copy link
Contributor

@appleby appleby left a comment

Choose a reason for hiding this comment

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

These changes look basically sane to me. Couple of small changes requested. I think getting this file down to zero errors can happen in a separate PR.

pyquil/paulis.py Outdated Show resolved Hide resolved
pyquil/paulis.py Outdated Show resolved Hide resolved
pyquil/paulis.py Outdated Show resolved Hide resolved
pyquil/paulis.py Show resolved Hide resolved
pyquil/paulis.py Outdated Show resolved Hide resolved
pyquil/paulis.py Outdated Show resolved Hide resolved
@appleby
Copy link
Contributor

appleby commented Jan 2, 2020

It looks like the travis build failed in make check-types. ~ 13 errors related to type changes here, mostly in the pyquil/experiment and pyquil/simulation modules.

Copy link
Contributor

@karalekas karalekas left a comment

Choose a reason for hiding this comment

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

@rht apply this diff to squash the remaining mypy errors, and fix the changelog conflict, and this is good to go

diff --git a/pyquil/experiment/_group.py b/pyquil/experiment/_group.py
index e0c9389..1103bbe 100644
--- a/pyquil/experiment/_group.py
+++ b/pyquil/experiment/_group.py
@@ -189,6 +189,7 @@ def _max_weight_operator(ops: Iterable[PauliTerm]) -> Union[None, PauliTerm]:
     mapping = dict()  # type: Dict[int, str]
     for op in ops:
         for idx, op_str in op:
+            assert isinstance(idx, int)
             if idx in mapping:
                 if mapping[idx] != op_str:
                     return None
diff --git a/pyquil/experiment/_main.py b/pyquil/experiment/_main.py
index a1b1ec8..ff790ff 100644
--- a/pyquil/experiment/_main.py
+++ b/pyquil/experiment/_main.py
@@ -286,7 +286,7 @@ class TomographyExperiment:
         """
         meas_qubits: Set[int] = set()
         for settings in self:
-            meas_qubits.update(settings[0].out_operator.get_qubits())
+            meas_qubits.update(cast(List[int], settings[0].out_operator.get_qubits()))
         return sorted(meas_qubits)
 
     def get_meas_registers(self, qubits: Optional[Sequence[int]] = None) -> List[int]:
@@ -377,8 +377,12 @@ class TomographyExperiment:
         """
         meas_qubits = self.get_meas_qubits()
 
-        in_pt = PauliTerm.from_list([(op, meas_qubits.index(q)) for q, op in setting.in_operator])
-        out_pt = PauliTerm.from_list([(op, meas_qubits.index(q)) for q, op in setting.out_operator])
+        in_pt = PauliTerm.from_list(
+            [(op, meas_qubits.index(cast(int, q))) for q, op in setting.in_operator]
+        )
+        out_pt = PauliTerm.from_list(
+            [(op, meas_qubits.index(cast(int, q))) for q, op in setting.out_operator]
+        )
 
         preparation_map = pauli_term_to_preparation_memory_map(in_pt)
         measurement_map = pauli_term_to_measurement_memory_map(out_pt)
diff --git a/pyquil/experiment/_memory.py b/pyquil/experiment/_memory.py
index ddf693d..0a54e58 100644
--- a/pyquil/experiment/_memory.py
+++ b/pyquil/experiment/_memory.py
@@ -15,7 +15,7 @@
 ##############################################################################
 
 import itertools
-from typing import Dict, List, Tuple
+from typing import Dict, List, Tuple, cast
 
 import numpy as np
 
@@ -102,7 +102,7 @@ def pauli_term_to_euler_memory_map(
     gamma_label = f"{prefix}_{suffix_gamma}"
 
     # assume the pauli indices are equivalent to the memory region
-    memory_size = max(term.get_qubits()) + 1
+    memory_size = max(cast(List[int], term.get_qubits())) + 1
 
     memory_map = {
         alpha_label: [0.0] * memory_size,
@@ -113,6 +113,7 @@ def pauli_term_to_euler_memory_map(
     tuples = {"X": tuple_x, "Y": tuple_y, "Z": tuple_z, "I": tuple_z}
 
     for qubit, operator in term:
+        assert isinstance(qubit, int)
         if operator not in tuples:
             raise ValueError(f"Unknown operator {operator}")
         memory_map[alpha_label][qubit] = tuples[operator][0]
diff --git a/pyquil/experiment/_setting.py b/pyquil/experiment/_setting.py
index 37c1bda..05186a7 100644
--- a/pyquil/experiment/_setting.py
+++ b/pyquil/experiment/_setting.py
@@ -123,7 +123,7 @@ def _pauli_to_product_state(in_state: PauliTerm) -> TensorProductState:
     else:
         return TensorProductState(
             [
-                _OneQState(label=pauli_label, index=0, qubit=qubit)
+                _OneQState(label=pauli_label, index=0, qubit=cast(int, qubit))
                 for qubit, pauli_label in in_state._ops.items()
             ]
         )
diff --git a/pyquil/simulation/_numpy.py b/pyquil/simulation/_numpy.py
index bd39c10..b275256 100644
--- a/pyquil/simulation/_numpy.py
+++ b/pyquil/simulation/_numpy.py
@@ -166,6 +166,7 @@ def _term_expectation(wf: np.ndarray, term: PauliTerm) -> Any:
     # Computes <psi|XYZ..XXZ|psi>
     wf2 = wf
     for qubit_i, op_str in term._ops.items():
+        assert isinstance(qubit_i, int)
         # Re-use QUANTUM_GATES since it has X, Y, Z
         op_mat = QUANTUM_GATES[op_str]
         wf2 = targeted_tensordot(gate=op_mat, wf=wf2, wf_target_inds=[qubit_i])
diff --git a/pyquil/simulation/_reference.py b/pyquil/simulation/_reference.py
index 437a3c1..30f1f83 100644
--- a/pyquil/simulation/_reference.py
+++ b/pyquil/simulation/_reference.py
@@ -30,6 +30,7 @@ def _term_expectation(wf: np.ndarray, term: PauliTerm, n_qubits: int) -> Any:
     # Computes <psi|XYZ..XXZ|psi>
     wf2 = wf
     for qubit_i, op_str in term._ops.items():
+        assert isinstance(qubit_i, int)
         # Re-use QUANTUM_GATES since it has X, Y, Z
         op_mat = QUANTUM_GATES[op_str]
         op_mat = lifted_gate_matrix(matrix=op_mat, qubit_inds=[qubit_i], n_qubits=n_qubits)

@karalekas karalekas added this to the v2.16 milestone Jan 6, 2020
@rht
Copy link
Contributor Author

rht commented Jan 6, 2020

@karalekas applied the diff in the 4th commit.

@rht
Copy link
Contributor Author

rht commented Jan 6, 2020

The CI failure is due to docs/source/migration.ipynb.

@karalekas
Copy link
Contributor

Figured out the issue -- nbformat v5.0.0 and v5.0.1 are broken, had to wait for them to fix it. Should be g2g soon.

@karalekas karalekas changed the title Precisify type hints of paulis.py Reduce mypy errors in paulis.py Jan 6, 2020
Copy link
Contributor

@appleby appleby left a comment

Choose a reason for hiding this comment

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

lgtm.

@karalekas karalekas merged commit a47c0ac into rigetti:master Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality 🎨 Improve code quality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants