Repository navigation
Fix TypedDict.get("missing_key") with string literal - #9906
Conversation
Previously this would throw an error, however according to PEP 589 .get() should be allowed for arbitrary expressions with type str, while only [] throws an error.
| reveal_type(test.pop(optional_keys, 3)) # N: Revealed type is 'Union[__main__.D, __main__.E, Literal[3]?]' | ||
| reveal_type(test.setdefault(good_keys, AAndB())) # N: Revealed type is 'Union[__main__.A, __main__.B]' | ||
| reveal_type(test.get(bad_keys)) # N: Revealed type is 'builtins.object*' | ||
| reveal_type(test.get(bad_keys, 3)) # N: Revealed type is 'builtins.object' |
There was a problem hiding this comment.
It would be great if a maintainer could have a look at this.
I don't know the difference between builtins.object and builtins.object* and couldn't find any reference to the asterisk in the documentation.
Therefor I just placed the needed asterisks by trial and error. This might or might not be wrong.
There was a problem hiding this comment.
The asterisk means that it's an inferred type; I think it can be safely ignored.
I'm not sure why mypy infers object here though, and it doesn't seem very useful.
There was a problem hiding this comment.
According to PEP 589 "The static type of d.get(e) should be object if the string value of e cannot be determined statically".
As far as I can tell PEP 589 doesn't say anything about d.get(e, default).
Also something else not related to my code seems to convert Union[..., builtins.object, ...] to just builtins.object, which can bee seen when making defauly.py line 234 retun Any instead. In that case mypy infers Union[__main__.A, Literal[3]?, Any].
There was a problem hiding this comment.
The default doesn't change anything, since we can't infer the type of the value from the default (the value type could be an arbitrary union type, for example).
Since typed dicts are heterogeneous, we can't predict the type of arbitrary keys. In this case the only possible precise type is object. Any would also work, but the spirit of PEP 484 is that there shouldn't be Any types in fully and precisely annotated programs. We don't want to silently produce an Any type if a programmer mistypes a string literal.
A union that includes object as an item is equivalent to object (it can contain arbitrary objects). Mypy sometimes simplifies these unions and it's okay.
There was a problem hiding this comment.
Thanks for the correction!
| reveal_type(d.get(a_key, u)) # N: Revealed type is 'Union[builtins.int, __main__.Unrelated]' | ||
| reveal_type(d.get(b_key, u)) # N: Revealed type is 'Union[builtins.str, __main__.Unrelated]' | ||
| d.get(c_key, u) # E: TypedDict "Outer" has no key 'c' | ||
| reveal_type(d.get(c_key, u)) # N: Revealed type is 'builtins.object' |
There was a problem hiding this comment.
Shouldn't this infer Unrelated, or at least a Union of something with Unrelated?
There was a problem hiding this comment.
Like in the other discussion, the type here would be Union[builtins.object, Unrelated], which mypy simplifies to builtins.object
| reveal_type(test.pop(optional_keys, 3)) # N: Revealed type is 'Union[__main__.D, __main__.E, Literal[3]?]' | ||
| reveal_type(test.setdefault(good_keys, AAndB())) # N: Revealed type is 'Union[__main__.A, __main__.B]' | ||
| reveal_type(test.get(bad_keys)) # N: Revealed type is 'builtins.object*' | ||
| reveal_type(test.get(bad_keys, 3)) # N: Revealed type is 'builtins.object' |
There was a problem hiding this comment.
The asterisk means that it's an inferred type; I think it can be safely ignored.
I'm not sure why mypy infers object here though, and it doesn't seem very useful.
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
Looks good. The original behavior was designed to catch some misspellings, but it was kind of inflexible. The new behavior is more flexible but the inferred object types could be confusing. It will be interesting to see what users think about this.
3 participants
Description
Fixes #9902
Previously this would throw an error, however according to PEP 589
.get() should be allowed for arbitrary expressions with type str, while
only [] throws an error.
Test Plan
All affected tests have been updated to the new, intended behavior. This is my first contribution to mypy, so it would be good to have an extra pair of eyes checking if everything is correct.