Skip to content

Fix/issue 1282 mypy batch3 part2#1386

Merged
mhucka merged 4 commits into
quantumlib:mainfrom
rosspeili:fix/issue-1282-mypy-batch3-part2
Jun 29, 2026
Merged

Fix/issue 1282 mypy batch3 part2#1386
mhucka merged 4 commits into
quantumlib:mainfrom
rosspeili:fix/issue-1282-mypy-batch3-part2

Conversation

@rosspeili

@rosspeili rosspeili commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

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

Typing-only, no runtime changes.

  • hamiltonians/special_operators.py, majorana_operator string parsing types, coefficient parameter annotation, number_operator return/Op types
  • contrib/representability/constraints/spin_orbital_2pos_constraints.py, fix incorrect Optional on g2d2map factor
  • contrib/representability/_dualbasis.py, correct scalar/bias/tensor_elements types (follow-imports dependency for spin_orbital constraints)

hartree_fock.py remains deferred, will do in next PR.

Test plan

  • strict mypy (ignore_errors=false, full follow-imports) on all 3 files
  • pytest on special_operators_test.py, spin_orbital_2pos_constraints_test.py, _dualbasis_test.py
  • black on changed files

Resolve type narrowing in majorana_operator string parsing and remove
incorrect Optional on g2d2map factor parameter.
Single-line g2d2map signature per black; use BosonOperator | FermionOperator
instead of typing.Union in special_operators.
Correct DualBasisElement scalar/bias/tensor types at the source and narrow
coefficient/Op types in special_operators so strict mypy passes with full
follow-imports, not only --follow-imports=skip.

@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 modernizes type annotations across several files by adopting PEP 604 union types (|) and standard collection types. The review feedback correctly points out that because the project targets Python 3.9, evaluating these new union types at runtime will raise a TypeError unless from __future__ import annotations is imported. The reviewer suggests adding this future import, simplifying redundant union types (e.g., using float instead of float | int), and annotating function parameters directly to eliminate the need for runtime typing.cast calls which would otherwise fail.

Comment thread src/openfermion/contrib/representability/_dualbasis.py
Comment thread src/openfermion/hamiltonians/special_operators.py Outdated
Comment thread src/openfermion/hamiltonians/special_operators.py Outdated
Comment thread src/openfermion/hamiltonians/special_operators.py
…).

Type the coefficient parameter directly in the signature so mypy no
longer needs typing.cast for the type-1 Majorana branch.
@rosspeili

Copy link
Copy Markdown
Contributor Author

Pushed a small hotfix based on suggestions, annotating coefficient: int | float | complex on majorana_operator and removed the typing.cast, but without the other changes.

Not applying the rest of the gemini suggestions:

  • from __future__ import annotations, OpenFermion requires Python 3.10+, we already use X | Y unions elsewhere (eg. plane_wave_hamiltonian) without __future__. The Python 3.9 concern doesn't apply here.
  • factor: float instead of float | int on g2d2map, cosmetic only, so keeping float | int for consistency with _dualbasis and prior Mypy reports many errors, and they should be fixed #1282 PRs.

Mypy + tests still pass locally on all 3 files. @mhucka let me know if I am missing something or if I should apply other suggestions too.

@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.

This looks good! Thank you.

@mhucka
mhucka added this pull request to the merge queue Jun 29, 2026
Merged via the queue into quantumlib:main with commit e70584f Jun 29, 2026
23 checks passed
@mhucka mhucka mentioned this pull request Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants