Fix final mypy errors for issue #1282 and enable strict checking#1392
Conversation
…cking Complete strict mypy for split_operator, linear_swap_network, and thc_jax; set ignore_errors to false in pyproject.toml; fix shell_tools_test duplicate kwarg.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables mypy type checking by setting ignore_errors = false in pyproject.toml and addresses numerous type annotation issues across the codebase. Specifically, it updates generator functions in Trotter algorithms to return Iterator[cirq.OP_TREE] instead of cirq.OP_TREE, introduces proper type casting for DiagonalCoulombHamiltonian, and modernizes type annotations in thc_jax.py using Python 3.9+ built-in collections and union operators. However, the review highlights a critical issue in thc_jax.py where calling np.asarray(xcur) on a JAX tracer object will raise a TracerArrayConversionError at runtime. To resolve this, it is recommended to pass xcur directly and update the type signature of unpack_thc_factors to accept npt.NDArray | jax.Array.
Cast isdf norm to float, pass list to json_normalize for pandas-stubs, and widen unpack_thc_factors for JAX tracers without np.asarray in the objective.
|
Pushed fixes on CI mypy (isdf float cast, data_types .tolist() for pandas-stubs), Gemini JAX tracing suggestion (unpack_thc_factors widened, no np.asarray in the objective), and black formatting on thc_jax.py. |
mhucka
left a comment
mhucka
left a comment
There was a problem hiding this comment.
Very minor tweak, otherwise it's good.
|
@rosspeili I took the liberty of making the 2 tiny changes and pushing to your branch, so that I can merge this PR today. |
|
Glad to see this merge @mhucka, thanks a lot for the patience and guidance as always <3 |
2 participants
Complete strict mypy for split_operator, linear_swap_network, and thc_jax, set ignore_errors to false in pyproject.toml, and fix shell_tools_test duplicate kwarg. Closes #1282