Skip to content

Fix mypy annotations in fermion_partitioning (#1282)#1349

Open
rosspeili wants to merge 1 commit into
quantumlib:mainfrom
rosspeili:fix/issue-1282-mypy
Open

Fix mypy annotations in fermion_partitioning (#1282)#1349
rosspeili wants to merge 1 commit into
quantumlib:mainfrom
rosspeili:fix/issue-1282-mypy

Conversation

@rosspeili

Copy link
Copy Markdown
Contributor

First slice for #1282: correct generator return types and local variable annotations in measurements/fermion_partitioning.py. No runtime or API behavior changes.

First slice for quantumlib#1282: correct generator return types and local variable annotations in measurements/fermion_partitioning.py. No runtime or API behavior changes.

@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 adds type annotations to several functions in src/openfermion/measurements/fermion_partitioning.py to improve type safety and code readability. Since there are no review comments, I have no feedback to provide.

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

Thank you for doing this!

There is one change I would recommend. Although Any is valid, it is better to define so-called generic parameter types, along the following lines:

from typing import TypeVar

T = TypeVar('T')

def pair_within(labels: list[T]) -> Generator[tuple[T, ...], None, None]:
    """Generates pairings of labels that contain each pair at least once."""
    ...

Applying this across other matching functions (like pair_between and pair_within_simultaneously) allows static analyzers to check that the output tuples contain elements of the exact same type as the input.

Would you be able to try to make the appropriate changes to this PR?

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