Adding type hints to circuit module #271
Conversation
| arg_label: Optional[str] = None, | ||
| index: Optional[List[int]] = None, | ||
| classical_controls: Optional[List[int]] = None, | ||
| control_value: None = None, |
There was a problem hiding this comment.
This should be
control_value: Optional[int] = None
I think
| classical_controls: Optional[List[int]] = None, | ||
| control_value: None = None, | ||
| classical_control_value: Optional[int] = None, | ||
| style: None = None, |
There was a problem hiding this comment.
This style was added in the rendering project right? The docstring is also missing. Would be nice to add the possible options here.
There was a problem hiding this comment.
Or just add a docstring pointing to the options we have.
| gates: Union[ | ||
| Tuple[Gate, Gate, Gate, Gate], | ||
| Tuple[Gate, Gate, Gate, Gate, Gate, Gate, Gate, Gate], | ||
| ], |
There was a problem hiding this comment.
This is definitely not right lol. It should be a list or tuple of gates.
| def add_circuit(self, qc, start=0, overwrite_user_gates=False): | ||
| def add_circuit( | ||
| self, | ||
| qc: "QubitCircuit", |
There was a problem hiding this comment.
Why does this use a string but not an imported class name?
| ): | ||
| state: Qobj, | ||
| cbits: Optional[List[int]] = None, | ||
| U_list: None = None, |
There was a problem hiding this comment.
This is a bug, U_list is no longer used. We can remove it.
| def initialize( | ||
| self, | ||
| state: Optional[Qobj] = None, | ||
| cbits: Optional[List[int]] = None, | ||
| measure_results: Optional[ | ||
| Union[List[int], Tuple[str, str], Tuple[str, str, str], Tuple[()]] | ||
| ] = None, |
There was a problem hiding this comment.
This is wrong again. I guess probably we should not return tuples with undetermined length in the code and should use a list instead? What do you think?
This should be list of int or str.
| measure_results: Optional[ | ||
| Union[List[int], Tuple[str, str], Tuple[str, str, str], Tuple[()]] | ||
| ] = None, |
| probabilities: Union[List[int], float64, int, List[float64]], | ||
| cbits: Optional[Union[List[List[int]], List[int], List[int64]]] = None, |
There was a problem hiding this comment.
The float64 and int64 are a bit weird. I don't see the reason.
PR Summary
This PR introduces type hints to the codebase using
MonkeyType. Initial applied to a single circuit module, and if feasible, can be extended to other modules. (Issue: #262)Encountered issues with
pytest decoratorswhile running through the test suite. This was resolved using pytest-monkeytype lib, which allowsMonkeyTypeto work correctly with pytest. Still requires a bit post processing, but it is minimal.QuTiP-QIP supports Python 3.9+, so we are unable to use Python 3.10+ syntax (e.g.,|forUnion). Also,MonkeyTypeis unable to generate in this format directly.Files Type Hint Status
_decompose.pybase_renderer.py(already present)circuit.pycircuitsimulator.pycolor_theme.py(not needed)mat_renderer.pyalready present)texrenderer.pytext_renderer.py(already present)