Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dev_tools/autogenerate-bloqs-notebooks-v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
python dev_tools/autogenerate-bloqs-notebooks-v2.py
"""

from typing import Iterable, List
from collections.abc import Iterable

from qualtran_dev_tools.bloq_finder import get_bloqdocspecs
from qualtran_dev_tools.jupyter_autogen import NotebookSpecV2, render_notebook
Expand Down Expand Up @@ -85,7 +85,7 @@ def render_notebooks():
render_notebook(nbspec)


def _get_toc_section_lines(caption: str, entries: List[str], maxdepth: int = 2) -> List[str]:
def _get_toc_section_lines(caption: str, entries: list[str], maxdepth: int = 2) -> list[str]:
"""Helper function to get the lines for a section of the table-of-contents."""
return (
['.. toctree::', f' :maxdepth: {maxdepth}', f' :caption: {caption}:', '']
Expand Down
6 changes: 3 additions & 3 deletions dev_tools/bloq-method-overrides-report.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
# 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 ForwardRef, Set, Type
from typing import ForwardRef

from qualtran_dev_tools.bloq_finder import get_bloq_classes

from qualtran import Bloq


def _call_graph(bc: Type[Bloq]):
def _call_graph(bc: type[Bloq]):
"""Check that a bloq class overrides the right call graph methods.

- Override `build_call_graph` with canonical type annotations.
Expand All @@ -42,7 +42,7 @@ def _call_graph(bc: Type[Bloq]):
)
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]

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.

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]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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?

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.

Huh. Let me look at it over the weekend.

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.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

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.

@micpap25 Finally got back to this.

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.

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

print(f"{bc}.build_call_graph -> 'BloqCountT'")


Expand Down
6 changes: 3 additions & 3 deletions dev_tools/bloq-quickstarter.html
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ <h3>Code</h3>
signature += "\n ])"

const template = `from functools import cached_property
from typing import Dict, Optional, Set, Union
from typing import Optional, Union

from attrs import frozen

Expand All @@ -238,10 +238,10 @@ <h3>Code</h3>
def signature(self) -> 'Signature':
${signature}

def build_composite_bloq(self, bb: 'BloqBuilder'${build_sig}) -> Dict[str, 'SoquetT']:
def build_composite_bloq(self, bb: 'BloqBuilder'${build_sig}) -> dict[str, 'SoquetT']:
raise NotImplementedError("Implement or delete.")

def bloq_counts(self, ssa: Optional['SympySymbolAllocator'] = None) -> Set['BloqCountT']:
def bloq_counts(self, ssa: Optional['SympySymbolAllocator'] = None) -> set['BloqCountT']:
raise NotImplementedError("Implement or delete.")

def short_name(self) -> str:
Expand Down
5 changes: 2 additions & 3 deletions dev_tools/dump-bloq-manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"""

from functools import cached_property
from typing import List, Tuple

from attrs import frozen
from qualtran_dev_tools.bloq_finder import get_bloq_classes, get_bloq_examples
Expand Down Expand Up @@ -70,7 +69,7 @@ def objectstring(self) -> str:
"""Returns the canonical string representation of the CObjectNode."""
return self.cobject_node.canonical_str()

def maybe_commented_out(self, be_column_width: int = 30) -> Tuple[str, str, str]:
def maybe_commented_out(self, be_column_width: int = 30) -> tuple[str, str, str]:
"""Generates a string representation for the manifest, potentially commented out.

This method checks if the object string is too long, unparsable, unloadable, or
Expand Down Expand Up @@ -121,7 +120,7 @@ def main():
names = sorted(bc._class_name_in_pkg_() for bc in bcs)

bes = get_bloq_examples()
items: List[BloqExampleListItem] = []
items: list[BloqExampleListItem] = []
for be in bes:
bloq = be.make()
try:
Expand Down
2 changes: 1 addition & 1 deletion dev_tools/qualtran_dev_tools/all_call_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import logging
import warnings
from typing import Iterable
from collections.abc import Iterable

import networkx as nx

Expand Down
29 changes: 15 additions & 14 deletions dev_tools/qualtran_dev_tools/bloq_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@
import importlib
import inspect
import subprocess
from collections.abc import Callable, Iterable
from pathlib import Path
from typing import Callable, Iterable, List, Optional, Tuple, Type
from typing import Optional

from qualtran import Bloq, BloqDocSpec, BloqExample

from .git_tools import get_git_root


def _get_git_paths(bloqs_root: Path, filter_func: Callable[[Path], bool]) -> List[Path]:
def _get_git_paths(bloqs_root: Path, filter_func: Callable[[Path], bool]) -> list[Path]:
"""Get only git-tracked *.py files based on `filter_func`."""
cp = subprocess.run(
['git', 'ls-files', '*.py'],
Expand All @@ -40,7 +41,7 @@ def _get_git_paths(bloqs_root: Path, filter_func: Callable[[Path], bool]) -> Lis

def _get_paths(
bloqs_root: Path, filter_func: Callable[[Path], bool], committed_only: bool = True
) -> List[Path]:
) -> list[Path]:
"""Get *.py files based on `filter_func`."""
if committed_only:
return _get_git_paths(bloqs_root, filter_func)
Expand All @@ -50,7 +51,7 @@ def _get_paths(
]


def get_bloq_module_paths(bloqs_root: Path, committed_only: bool = True) -> List[Path]:
def get_bloq_module_paths(bloqs_root: Path, committed_only: bool = True) -> list[Path]:
"""Get *.py files for non-test, non-init modules under `bloqs_root`."""

def is_module_path(path: Path) -> bool:
Expand All @@ -65,7 +66,7 @@ def is_module_path(path: Path) -> bool:
return _get_paths(bloqs_root, is_module_path, committed_only=committed_only)


def get_bloq_test_module_paths(bloqs_root: Path, committed_only: bool = True) -> List[Path]:
def get_bloq_test_module_paths(bloqs_root: Path, committed_only: bool = True) -> list[Path]:
"""Get *_test.py files under `bloqs_root`."""

def is_test_module_path(path: Path) -> bool:
Expand All @@ -82,7 +83,7 @@ def _bloq_modpath_to_modname(path: Path) -> str:
return 'qualtran.bloqs.' + str(path)[: -len('.py')].replace('/', '.')


def modpath_to_bloqs(path: Path) -> Iterable[Type[Bloq]]:
def modpath_to_bloqs(path: Path) -> Iterable[type[Bloq]]:
"""Given a module path, return all the `Bloq` classes defined within."""
modname = _bloq_modpath_to_modname(path)
mod = importlib.import_module(modname)
Expand All @@ -100,7 +101,7 @@ def modpath_to_bloqs(path: Path) -> Iterable[Type[Bloq]]:
yield cls


def modpath_to_bloq_exs(path: Path) -> Iterable[Tuple[str, str, BloqExample]]:
def modpath_to_bloq_exs(path: Path) -> Iterable[tuple[str, str, BloqExample]]:
"""Given a module path, return all the `BloqExample`s defined within."""
modname = _bloq_modpath_to_modname(path)
mod = importlib.import_module(modname)
Expand All @@ -109,7 +110,7 @@ def modpath_to_bloq_exs(path: Path) -> Iterable[Tuple[str, str, BloqExample]]:
yield modname, name, obj


def modpath_to_bloqdocspecs(path: Path) -> Iterable[Tuple[str, str, BloqDocSpec]]:
def modpath_to_bloqdocspecs(path: Path) -> Iterable[tuple[str, str, BloqDocSpec]]:
"""Given a module path, return all the `BloqDocSpec`s defined within."""
modname = _bloq_modpath_to_modname(path)
mod = importlib.import_module(modname)
Expand All @@ -118,44 +119,44 @@ def modpath_to_bloqdocspecs(path: Path) -> Iterable[Tuple[str, str, BloqDocSpec]
yield modname, name, obj


def get_bloq_classes(bloqs_root: Optional[Path] = None) -> List[Type[Bloq]]:
def get_bloq_classes(bloqs_root: Optional[Path] = None) -> list[type[Bloq]]:
committed_only = bloqs_root is None
if bloqs_root is None:
reporoot = get_git_root()
bloqs_root = reporoot / 'qualtran/bloqs'

paths = get_bloq_module_paths(bloqs_root, committed_only=committed_only)
bloq_clss: List[Type[Bloq]] = []
bloq_clss: list[type[Bloq]] = []
for path in paths:
bloq_clss.extend(modpath_to_bloqs(path))
return bloq_clss


def get_bloq_examples(bloqs_root: Optional[Path] = None) -> List[BloqExample]:
def get_bloq_examples(bloqs_root: Optional[Path] = None) -> list[BloqExample]:
committed_only = bloqs_root is None
if bloqs_root is None:
reporoot = get_git_root()
bloqs_root = reporoot / 'qualtran/bloqs'

paths = get_bloq_module_paths(bloqs_root, committed_only=committed_only)

bexamples: List[BloqExample] = []
bexamples: list[BloqExample] = []
for path in paths:
for modname, name, be in modpath_to_bloq_exs(path):
bexamples.append(be)

return bexamples


def get_bloqdocspecs(bloqs_root: Optional[Path] = None) -> List[BloqDocSpec]:
def get_bloqdocspecs(bloqs_root: Optional[Path] = None) -> list[BloqDocSpec]:
committed_only = bloqs_root is None
if bloqs_root is None:
reporoot = get_git_root()
bloqs_root = reporoot / 'qualtran/bloqs'

paths = get_bloq_module_paths(bloqs_root, committed_only=committed_only)

bdspecs: List[BloqDocSpec] = []
bdspecs: list[BloqDocSpec] = []
for path in paths:
for modname, name, bds in modpath_to_bloqdocspecs(path):
bdspecs.append(bds)
Expand Down
17 changes: 9 additions & 8 deletions dev_tools/qualtran_dev_tools/bloq_report_card.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
# limitations under the License.
import time
import warnings
from typing import Any, Dict, Iterable, List, Optional, Set, Type
from collections.abc import Iterable
from typing import Any, Optional

import pandas as pd
import pandas.io.formats.style
Expand All @@ -31,7 +32,7 @@
from .bloq_finder import get_bloq_classes, get_bloq_examples


def _get_package(bloq_cls: Type[Bloq]) -> str:
def _get_package(bloq_cls: type[Bloq]) -> str:
"""The package name for a bloq class"""
return '.'.join(bloq_cls.__module__.split('.')[:-1])

Expand All @@ -56,8 +57,8 @@ def format_status(v: BloqCheckResult):


def bloq_classes_with_no_examples(
bclasses: Iterable[Type[Bloq]], bexamples: Iterable[BloqExample]
) -> Set[Type[Bloq]]:
bclasses: Iterable[type[Bloq]], bexamples: Iterable[BloqExample]
) -> set[type[Bloq]]:
ks = set(bclasses)
for be in bexamples:
try:
Expand All @@ -72,13 +73,13 @@ def bloq_classes_with_no_examples(
CHECKCOLS = ['make', 'decomp', 'counts', 'serialize', 'qtyping']


def record_for_class_with_no_examples(k: Type[Bloq]) -> Dict[str, Any]:
def record_for_class_with_no_examples(k: type[Bloq]) -> dict[str, Any]:
return {'bloq_cls': k.__name__, 'package': _get_package(k), 'name': '-'} | {
check_name: BloqCheckResult.MISSING for check_name in CHECKCOLS
}


def record_for_bloq_example(be: BloqExample) -> Dict[str, Any]:
def record_for_bloq_example(be: BloqExample) -> dict[str, Any]:
start = time.perf_counter()
record = {
'bloq_cls': be.bloq_cls.__name__,
Expand All @@ -101,7 +102,7 @@ def show_bloq_report_card(df: pd.DataFrame) -> pandas.io.formats.style.Styler:


def get_bloq_report_card(
bclasses: Optional[Iterable[Type[Bloq]]] = None,
bclasses: Optional[Iterable[type[Bloq]]] = None,
bexamples: Optional[Iterable[BloqExample]] = None,
package_prefix: str = 'qualtran.bloqs.',
) -> pd.DataFrame:
Expand All @@ -115,7 +116,7 @@ def get_bloq_report_card(
skips = ['qubitization_qpe_hubbard_model_small', 'qubitization_qpe_hubbard_model_large']
bexamples = [bex for bex in bexamples if bex.name not in skips]

records: List[Dict[str, Any]] = []
records: list[dict[str, Any]] = []
missing_bclasses = bloq_classes_with_no_examples(bclasses, bexamples)
records.extend(record_for_class_with_no_examples(k) for k in missing_bclasses)
records.extend(record_for_bloq_example(be) for be in bexamples)
Expand Down
4 changes: 2 additions & 2 deletions dev_tools/qualtran_dev_tools/clean_notebooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
import subprocess
from pathlib import Path
from tempfile import NamedTemporaryFile
from typing import Any, List
from typing import Any

import nbformat
from nbconvert.preprocessors import ClearMetadataPreprocessor, ClearOutputPreprocessor


def get_nb_rel_paths(rootdir) -> List[Path]:
def get_nb_rel_paths(rootdir) -> list[Path]:
"""List all checked-in *.ipynb files within `rootdir`."""
cp = subprocess.run(
['git', 'ls-files', '*.ipynb'],
Expand Down
12 changes: 6 additions & 6 deletions dev_tools/qualtran_dev_tools/incremental_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import os.path
import re
from typing import cast, Dict, List, Optional, Set, Tuple
from typing import cast, Optional

from . import shell_tools
from .prepared_env import PreparedEnv
Expand Down Expand Up @@ -57,7 +57,7 @@
EXPLICIT_OPT_OUT_COMMENT = "#coverage:ignore"


def diff_to_new_interesting_lines(unified_diff_lines: List[str]) -> Dict[int, str]:
def diff_to_new_interesting_lines(unified_diff_lines: list[str]) -> dict[int, str]:
"""Extracts a set of 'interesting' lines out of a GNU unified diff format.

Format:
Expand Down Expand Up @@ -124,7 +124,7 @@ def fix_line_from_coverage_file(line):

def get_incremental_uncovered_lines(
abs_path: str, base_commit: str, actual_commit: Optional[str]
) -> List[Tuple[int, str, str]]:
) -> list[tuple[int, str, str]]:
"""Find touched but uncovered lines in the given file.

Uses git diff and the annotation files created by `pytest --cov-report annotate` to find
Expand Down Expand Up @@ -190,9 +190,9 @@ def line_content_counts_as_uncovered_manual(content: str) -> bool:
return True


def determine_ignored_lines(content: str) -> Set[int]:
def determine_ignored_lines(content: str) -> set[int]:
lines = content.split("\n")
result: List[int] = []
result: list[int] = []

i = 0
while i < len(lines):
Expand Down Expand Up @@ -222,7 +222,7 @@ def determine_ignored_lines(content: str) -> Set[int]:
return {e + 1 for e in result}


def naive_find_end_of_scope(lines: List[str], i: int) -> int:
def naive_find_end_of_scope(lines: list[str], i: int) -> int:
# TODO: deal with line continuations, which may be less indented.
# Github issue: https://github.com/quantumlib/Cirq/issues/2968
line = lines[i]
Expand Down
Loading
Loading