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

Match not type guarding TypedDicts #4248

Closed
alex-way opened this issue Apr 17, 2023 · 9 comments
Closed

Match not type guarding TypedDicts #4248

alex-way opened this issue Apr 17, 2023 · 9 comments
Assignees

Comments

@alex-way
Copy link

Environment data

  • Language Server version: v2023.4.20
  • OS and version: Windows 10
  • Python version (& distribution if applicable, e.g. Anaconda): Python v3.11.3

Code Snippet

from typing import Literal, TypedDict, Union


class Base(TypedDict):
    value: int


class Derived(Base):
    name: Literal["foo"]
    extra_value: int


class MegaDerived(Base):
    name: Literal["bar"]
    other_extra_value: int


Things = Union[Derived, MegaDerived]

my_list = [Derived(name="foo", value=1, extra_value=2), MegaDerived(name="bar", value=3, other_extra_value=5)]

for item in my_list:
    match item["name"]:
        case "foo":
            print(
                item["extra_value"]
            )  # Could not access item in TypedDict⏎  "extra_value" is not a defined key in "MegaDerived"
        case "bar":
            print(
                item["other_extra_value"]
            )  # Could not access item in TypedDict⏎  "other_extra_value" is not a defined key in "Derived"

Repro Steps

  1. Create and view the file in vscode

Expected behavior

Line 26 should type guard item so that it's of type Derived and line 30 should type guard item so that it's of type MegaDerived

Actual behavior

The type hint for item remains unchanged as Derived | MegaDerived

Logs

XXX
@erictraut
Copy link
Contributor

To repro this, I need to enable strict mode type checking, or at least enable strictListInference. Without this, the inferred type of my_list is list[Unknown] and no type errors are reported.

The current behavior is by design. Pyright, the type checker that underlies pylance, performs type narrowing of the subject expression in match statements. In this case, the subject expression is item["name"], and its initial (un-narrowed) type is Literal["foo", "bar"]. Its type is properly narrowed by the match statement. You can see this if you change your sample to the following:

for item in my_list:
    match item["name"]:
        case "foo" as x:
            reveal_type(x) # Literal["foo"]
        case "bar" as y:
            reveal_type(y) # Literal["bar"]

Your code is relying on narrowing of item, which is not the subject expression; it's a subexpression within that expression. This is not currently supported. I'll note that this form of type narrowing isn't supported by other type checkers either, including mypy.

We could look at adding such support in the future if there was sufficient demand, but I suspect that it would be expensive from an analysis standpoint. If it affects analysis performance in the general case, it wouldn't be a good tradeoff to support this particular pattern.

Pyright does support TypedDict discrimination when using certain forms if conditions. You can change your code to the following, and it will narrow the type of the subexpression.

for item in my_list:
    if item["name"] == "foo":
        print(item["extra_value"])
    elif item["name"] == "bar":
        print(item["other_extra_value"])

@alex-way
Copy link
Author

Thank you for the thorough reply Eric 😄

I did initially start with an if/else block which was mostly working for my use case but I wanted the exhaustive handling that a match statement offers, hence raising this request. I had thought that the same type checking logic would be applied to the match statement as the if/else.

I'm happy to close if you think this is intended behaviour!

@erictraut
Copy link
Contributor

If you want to want to use a match statement, switching to a mapping pattern would work fine:

for item in my_list:
    match item:
        case {"name": "foo"}:
            print(item["extra_value"])
        case {"name": "bar"}:
            print(item["other_extra_value"])

Let's leave the issue open for now. I'll give your feature request a bit more thought.

@alex-way
Copy link
Author

Oh, that's really interesting! I'll have to brush up on my knowledge of match statements but that does indeed work 😃

@erictraut
Copy link
Contributor

BTW, if you want to use exhaustive matching with if/else, you can use the assert_never function, which was added to the typing module in Python 3.11 (and is in typing_extensions for backward compatibility).

for item in my_list:
    if item["name"] == "foo":
        print(item["extra_value"])
    elif item["name"] == "bar":
        print(item["other_extra_value"])
    else:
        assert_never(item)

@alex-way
Copy link
Author

That's actually super helpful, thank you once again!

erictraut pushed a commit to microsoft/pyright that referenced this issue Apr 19, 2023
…ject expression for match statements. In particular, added support for discriminated TypedDicts and tuples that are discriminated on literals. This addresses microsoft/pylance-release#4248.
@erictraut
Copy link
Contributor

I figured out a way to handle this is a way that doesn't have a measurable impact on performance. It will be included in the next release of pyright and a future release of pylance.

@alex-way
Copy link
Author

You wizard, my code thanks you for the quick turnaround 😄

@alex-way
Copy link
Author

Pylance 2023.4.31 includes pyright 1.1.304 which fixes this issue.

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

No branches or pull requests

3 participants