Skip to content

Fix mypy errors in batch 2 part 3 (#1282)#1365

Merged
mhucka merged 4 commits into
quantumlib:mainfrom
rosspeili:fix/issue-1282-mypy-batch2-part3
Jun 21, 2026
Merged

Fix mypy errors in batch 2 part 3 (#1282)#1365
mhucka merged 4 commits into
quantumlib:mainfrom
rosspeili:fix/issue-1282-mypy-batch2-part3

Conversation

@rosspeili

Copy link
Copy Markdown
Contributor

plane_wave_hamiltonian: annotate spins as list[Optional[int]]. physical_costing: MagicStateFactory rounds and failure_rate are floats at runtime.

plane_wave_hamiltonian: annotate spins as list[Optional[int]]. physical_costing: MagicStateFactory rounds and failure_rate are floats at runtime.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates type annotations across two files, changing the types of rounds and failure_rate from int to float in physical_costing.py, and adding a type annotation for spins in plane_wave_hamiltonian.py. The reviewer pointed out that using lowercase list for type subscripting in plane_wave_hamiltonian.py could lead to a runtime TypeError on Python versions older than 3.9, and suggested using List from the typing module instead to ensure compatibility and consistency.

Comment thread src/openfermion/hamiltonians/plane_wave_hamiltonian.py Outdated
Comment thread src/openfermion/hamiltonians/plane_wave_hamiltonian.py Outdated

@mhucka mhucka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please check the type annotation in plane_wave_hamiltonians.py

@mhucka mhucka added area/health Involves code and/or project health area/python Involves Python code labels Jun 17, 2026
@mhucka mhucka mentioned this pull request Jun 17, 2026
Per review: use list[...] instead of typing.List (3.10+).
@rosspeili

Copy link
Copy Markdown
Contributor Author

@mhucka thanks and agreed, switched back to list[Optional[int]] and updated the other List[...] annotations in this file to list[...] too similar to previous PRs.

@rosspeili rosspeili requested a review from mhucka June 17, 2026 07:21

@mhucka mhucka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for working on this. I recommend some tweaks below.

Comment thread src/openfermion/hamiltonians/plane_wave_hamiltonian.py Outdated
Comment thread src/openfermion/hamiltonians/plane_wave_hamiltonian.py Outdated
Comment thread src/openfermion/hamiltonians/plane_wave_hamiltonian.py Outdated
Comment thread src/openfermion/hamiltonians/plane_wave_hamiltonian.py Outdated
Comment thread src/openfermion/hamiltonians/plane_wave_hamiltonian.py Outdated
@mhucka mhucka self-assigned this Jun 18, 2026
@mhucka mhucka added the priority/before-1.7.2 Things to do before the next release label Jun 18, 2026
Use builtin list/tuple, X | None, and int | float throughout plane_wave_hamiltonian. Add operator: FermionOperator | None. Remove typing import per mhucka review.
@rosspeili

Copy link
Copy Markdown
Contributor Author

@mhucka thanks for the tip, and sorry for not coming up with this myself, applied all tweaks in latest push, also removed the typing import. Let me know how this looks.

@mhucka mhucka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great, thanks for doing this!

@mhucka mhucka added this pull request to the merge queue Jun 21, 2026
Merged via the queue into quantumlib:main with commit 547060d Jun 21, 2026
22 checks passed
@rosspeili rosspeili deleted the fix/issue-1282-mypy-batch2-part3 branch June 22, 2026 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/health Involves code and/or project health area/python Involves Python code priority/before-1.7.2 Things to do before the next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants