Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mypyc] Support iterating over a TypedDict #14747

Merged
merged 5 commits into from
Mar 20, 2023

Conversation

ichard26
Copy link
Collaborator

An optimization to make iterating over dict.keys(), dict.values() and dict.items() faster caused mypyc to crash while compiling a TypedDict. This commit fixes Builder.get_dict_base_type to properly handle TypedDictType.

Fixes mypyc/mypyc#869.

An optimization to make iterating over dict.keys(), dict.values() and
dict.items() faster caused mypyc to crash while compiling a TypedDict.
This commit fixes `Builder.get_dict_base_type` to properly handle
TypedDictType.
@@ -50,6 +50,9 @@ class _TypedDict(Mapping[str, object]):
# Mypy expects that 'default' has a type variable type.
def pop(self, k: NoReturn, default: _T = ...) -> object: ...
def update(self: _T, __m: _T) -> None: ...
def items(self) -> dict_items[str, object]: ...
def keys(self) -> dict_keys[str, object]: ...
def values(self) -> dict_values[str, object]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

The return types aren't imported anywhere. For some reason we don't generate errors even though these are undefined. Maybe we are ignoring errors in test stubs?

You should be able to use a less specific type. These are adapted from mypyc test stubs:

    def keys(self) -> Iterable[str]: pass
    def values(self) -> Iterable[object]: pass
    def items(self) -> Iterable[Tuple[str, object]]: pass

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, good catch! And yeah I have no idea what's up with the lack of errors. I opened an issue to note that we should investigate this later: mypyc/mypyc#976.

@ichard26 ichard26 requested a review from JukkaL February 23, 2023 20:26
@@ -50,6 +50,9 @@ class _TypedDict(Mapping[str, object]):
# Mypy expects that 'default' has a type variable type.
def pop(self, k: NoReturn, default: _T = ...) -> object: ...
def update(self: _T, __m: _T) -> None: ...
def items(self) -> Iterable[str, object]: ...
def keys(self) -> Iterable[str, object]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

The return types are still invalid. It seems quite important to report errors in test stubs, as otherwise tests might not be testing what we expect them to.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

This looks good now! Could you also add a run test? The generated IR is pretty complex so it's a bit tricky to validate that it works right.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! Looks good.

@JukkaL JukkaL merged commit 9944d5f into python:master Mar 20, 2023
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.

Using TypedDict in function triggers AssertionError during compilation
2 participants