`dev-tools` replace deprecated `typing` aliases with those from `collections` by micpap25 · Pull Request #1851 · quantumlib/Qualtran

micpap25 · 2026-05-08T14:20:28Z

Partial on #1653
As suggested by @mhucka isolate the changes from #1677 to smaller subsets of the codebase so that the review process is easier.
This PR removes compatibility with pre-3.10 python versions, but Qualtran isn't intended to target those anymore.

Remove pre-3.10 python version targeting, and for the dev-tools folder, update typing classes to modern standards.

gemini-code-assist

Code Review

This pull request updates type annotations across the codebase to use built-in collection types and collections.abc equivalents, replacing the older typing module generics. It also simplifies imports in protocol buffer interface files by removing conditional typing_extensions logic in favor of the standard typing module. I have no feedback to provide as there were no review comments.

mhucka

Thanks for this work! It's quite thorough.

I found one place where there's a subtle bug, and a few suggestions for minor tweaks elsewhere, but otherwise this is almost done!

mhucka · 2026-05-13T22:33:09Z

    if annot['ssa'] != 'SympySymbolAllocator':
        print(f"{bc}.build_call_graph `ssa: 'SympySymbolAllocator'`")
-    if annot['return'] != Set[ForwardRef('BloqCountT')]:  # type: ignore[misc]
+    if annot['return'] != set[ForwardRef('BloqCountT')]:  # type: ignore[misc]


This one is incorrect. The typing.Set and the built-in set are not considered equal when used in type annotations, even if they contain the same arguments. Here is a small test program to demonstrate:

from typing import ForwardRef, Set # Original annotation using typing.Set original = Set[ForwardRef('BloqCountT')] # PEP 585 annotation using built-in set pep585 = set[ForwardRef('BloqCountT')] print(f"Is {original} == {pep585}?") print(f"Result: {original == pep585}")

In the typing module, parameterizing a type with a string automatically wraps that string in a ForwardRef object. That meant that typing.Set['BloqCountT'] was identical to typing.Set[typing.ForwardRef('BloqCountT')] in the previous version of the code. However, PEP 585's built-in collections do not auto-convert strings to ForwardRef objects. When it evaluates set['BloqCountT'], Python directly creates a types.GenericAlias containing the literal string 'BloqCountT'.

The fix in this particular case turns out to be easy: get rid of the ForwardRef completely and do:

if annot['return'] != set['BloqCountT']: # type: ignore[misc]

Good to know, because I replace typing.Set with set throughout the other parts of the code; I could go and fix that as well / remove the ForwardRef usage there too? I will make the suggested fix, though.

Regarding this bug, it seems that making the change you recommended causes an issue in the dev-tools CI about BloqCountT being undefined. It seems fixing this for every type typing.Set is changed to the built-in set will require a lot of TYPE_CHECKING imports?

Huh. Let me look at it over the weekend.

Regarding this bug, it seems that making the change you recommended causes an issue in the dev-tools CI about BloqCountT being undefined. It seems fixing this for every type typing.Set is changed to the built-in set will require a lot of TYPE_CHECKING imports?

I don't have an answer to this yet, but I see all the other comments have been resolved (thank you), so what's left is this question and the .pyi files question above.

Just following up to see if you've had a chance to test this? Thanks!

@micpap25 Finally got back to this.

Regarding this bug, it seems that making the change you recommended causes an issue in the dev-tools CI about BloqCountT being undefined. It seems fixing this for every type typing.Set is changed to the built-in set will require a lot of TYPE_CHECKING imports?

In the current PR, the change does not seem to be that bad. I did it locally to test:

diff --git a/dev_tools/bloq-method-overrides-report.py b/dev_tools/bloq-method-overrides-report.py index 77f51231..836f342a 100644 --- a/dev_tools/bloq-method-overrides-report.py +++ b/dev_tools/bloq-method-overrides-report.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Set from qualtran_dev_tools.bloq_finder import get_bloq_classes @@ -45,7 +45,7 @@ def _call_graph(bc: type[Bloq]): ) if annot['ssa'] != 'SympySymbolAllocator': print(f"{bc}.build_call_graph `ssa: 'SympySymbolAllocator'`") - if annot['return'] != set['BloqCountT']: # type: ignore[misc] + if annot['return'] != Set['BloqCountT']: # type: ignore[misc] print(f"{bc}.build_call_graph -> 'BloqCountT'") diff --git a/dev_tools/bloq-quickstarter.html b/dev_tools/bloq-quickstarter.html index 7ae2240a..d7f16cad 100644 --- a/dev_tools/bloq-quickstarter.html +++ b/dev_tools/bloq-quickstarter.html @@ -215,7 +215,7 @@ signature += "\n ])" const template = `from functools import cached_property -from typing import Optional, Union +from typing import Optional, Union, Set from attrs import frozen

mhucka

@micpap25 Sorry for the delay in getting back to this. I tested it and mypy and pytest are all happy in my Python 3.12 environment.

To make the code cleaner and avoid having to wrap type annotations in string quotes like 'Writable', we should add from __future__ import annotations to the top of the files that use Writable.

By putting that as the first instruction in the files, Python postpones evaluation of all type annotations at runtime. This allows you to write unquoted type hints. Illustration:

from __future__ import annotations
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from .._linking_writer import Writable

# Note that Writable now does not need to be quoted.
def _write_text(f: Writable, part: DocstringSectionText, level: int) -> None:

mhucka · 2026-06-10T04:53:32Z

@micpap25 Thank you for your patience with this. I think we have a possible path forward; I'm just waiting for confirmation from the others, which I hope we'll have in 24 hrs.

To make the code cleaner and avoid having to wrap type annotations in string quotes like `'Writable'``, we can add `from __future__ import annotations` to the top of the files that use `Writable`. By putting that as the first instruction in the files, Python postpones evaluation of all type annotations at runtime. This allows us to write unquoted type hints.

mhucka · 2026-06-13T03:38:59Z

Update: we'll do a separate PR to run the proto building script after this one is merged.

Parameterizing the built-in set with a string (e.g. `set['BloqCountT']`) evaluates to a `types.GenericAlias(set, 'BloqCountT')` which contains the literal string, rather than wrapping it with a `ForwardRef` object like `typing.Set` does.

There is a separate script for checking dev_tools.

mhucka · 2026-06-13T04:11:08Z

@micpap25 I hope you don't mind; I went ahead and pushed changes directly to your branch with the change to Set as well as with the latest changes from the main branch from the upstream Qualtran repo.

If you are satisfied with the changes, I can merge the PR.

micpap25 · 2026-06-13T12:36:05Z

Yep, thank you for finishing it off!

mhucka · 2026-06-22T05:00:00Z

Update: PR #1886 is a follow-up to the comment above about doing a separate PR to run the build-protos.sh script.

This is a follow-up to PR #1851. This PR consists solely of the results of running `dev_tools/build-protos.sh`.

Partially implement changing typing to collections.abc

bf8180c

Remove pre-3.10 python version targeting, and for the dev-tools folder, update typing classes to modern standards.

gemini-code-assist Bot reviewed May 8, 2026

View reviewed changes

micpap25 mentioned this pull request May 13, 2026

replace deprecated typing aliases with those from collections #1653

Open

Merge branch 'main' into typing-update-dev-tools

f8a5b55

mhucka requested changes May 13, 2026

View reviewed changes

micpap25 added 10 commits May 15, 2026 17:58

Merge branch 'main' into typing-update-dev-tools

1521507

Update based on suggestions

1edefb0

format fix

9ff829e

Try the other way

c132c4b

forgot a file, try this instead

3194ef6

forgot to save changes

71dd583

Import for type checking

3ad036b

format fix

5dc77d1

another format fix

94aa361

Merge branch 'main' into typing-update-dev-tools

9dcf16e

mhucka requested changes Jun 3, 2026

View reviewed changes

Comment thread qualtran/protos/bloq_pb2.pyi

mhucka added 2 commits June 12, 2026 18:39

Make check/mypy also check dev_tools/

8b2f973

mhucka added 3 commits June 13, 2026 03:56

Go back to using typing.Set

7ec3017

Parameterizing the built-in set with a string (e.g. `set['BloqCountT']`) evaluates to a `types.GenericAlias(set, 'BloqCountT')` which contains the literal string, rather than wrapping it with a `ForwardRef` object like `typing.Set` does.

chore: merge branch

51175ce

Run through check/format-incremental --apply

111e378

mhucka approved these changes Jun 13, 2026

View reviewed changes

Revert change to check/mypy

8eb62f3

There is a separate script for checking dev_tools.

mhucka merged commit d7550a4 into quantumlib:main Jun 15, 2026
21 of 22 checks passed

mhucka mentioned this pull request Jun 22, 2026

Rebuild protos using build-protos.sh #1886

Merged

mhucka added a commit that referenced this pull request Jun 22, 2026

Rebuild protos using build-protos.sh (#1886)

279d42c

This is a follow-up to PR #1851. This PR consists solely of the results of running `dev_tools/build-protos.sh`.

Conversation

micpap25 commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

mhucka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhucka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mhucka commented Jun 10, 2026

Uh oh!

mhucka commented Jun 13, 2026

Uh oh!

mhucka commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

micpap25 commented Jun 13, 2026

Uh oh!

Uh oh!

mhucka commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

micpap25 commented May 8, 2026 •

edited

Loading

mhucka commented Jun 13, 2026 •

edited

Loading