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

Improvements to .get(...) on TypedDict with Literals #3292

Closed
bijij opened this issue Apr 2, 2022 · 3 comments
Closed

Improvements to .get(...) on TypedDict with Literals #3292

bijij opened this issue Apr 2, 2022 · 3 comments
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request

Comments

@bijij
Copy link

bijij commented Apr 2, 2022

Is your feature request related to a problem? Please describe.
This is somewhat related to #1058

I have a similar situation where i have multiple TypedDicts in a union, and only one of which holds a key.

Something like:

# pyright: strict

from typing import Optional, TypedDict, Union


class A(TypedDict):
    foo: int


class B(TypedDict):
    bar: str

C = Union[A, B]


def test(c: C) -> Optional[int]:
    return c.get('foo')

Which type-checks fine, as it should, however we'll find if we change the return type of test to anything (bar NoReturn) the code will still type-check, even though I would assume most developers would expect an error to occur.

I understand how pyright currently handles the situation, falling back to the (str) -> Any overload, if the key isn't matched in a part of the union.

So in the example case c.get('foo'):

  • matches (k: Literal['foo']) -> int | None for A (this in itself seems like another thing which could be improved? if the TypedDict is total or the key is Required this can't be None)
  • and (k: str) -> Any | None for B
    which flatten to int | Any | None

Describe the solution you'd like
Is there any reason why if the TypedDict is total and the argument to .get is a Literal it makes sense for the str fallback to be reached in the first place? if that was omitted we'd end up with the expected int | None here.

There may be some case where subclassing of the types would cause issues? as a counter to that I would wonder, if the previous statement isn't true whether decorating A and B with @final would alleviate the issue?

There's possibly some other argument to be had about Any being used here at all, since if the types of all keys are known, .get(...) really should only be able to return a Union of those value types and None.

Thoughts?

@bijij bijij added the enhancement request New feature or request label Apr 2, 2022
@bijij bijij changed the title Improvements to .get(...) on Unions of TypedDict with Literals Improvements to .get(...) on TypedDict with Literals Apr 2, 2022
@erictraut
Copy link
Collaborator

TypedDicts can be subclassed, so even if a TypedDict doesn't define a key, one of its subclasses might. The only way around this is to mark the TypedDict as @final, which is possible but rarely used. Most of the suggestions you're proposing could potentially apply, but only to @final TypedDicts.

I can't think of any other case where the method signatures for a class depend on whether the class is marked @final or not, so synthesizing different methods based on finality would be an odd special case here. I'm not opposed to it, but I need to think more about whether there would be negative consequences.

There needs to be some sort of a fallback to support non-literal str values passed to get.

Prior to PEP 675, there was no way in the type system to specify "any literal string value". Now that we have that capability, I could theoretically add another overload signature that returns None for any literal string that is not a known key. But once again, this is possible only for a TypedDict that is marked @final.

Returning a union of all possible value types goes against a principle used throughout the typeshed stdlib stubs. There has been some talk about adding a "weak union" or an AnyOf type operator that might apply here. These discussions haven't bottomed out though.

@erictraut
Copy link
Collaborator

I've updated the logic to honor required fields in the "match" case. I've also added code to handle the "not match" case for a literal string that doesn't correspond to a known field if the TypedDict is marked @final.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Apr 2, 2022
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.235, which I just pblished. It will also be included in the next release of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants