| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
This issue is nuanced, because many unpacking behaviors in Python are unsafe. If you unpack a list[int] into a function that takes (int, int) (but not *args: int), every type checker allows that, even though the list could obviously contain fewer or more than two elements. The collective decision seems to be that since length is not part of what the list type encodes, we trust the user rather than erroring. In ty we made the decision not to error on the first case here, because we saw large mypy-primer fallout if we enforced this rule, and we felt it was analogous to the case of list[int] -- an open TypedDict may contain extra elements (just like a list may contain any number of elements), but the type doesn't explicitly say that it does, so we can be forgiving here. We decided to be stricter with extra_items, in part because there is no widespread usage yet, so usage will adapt to what type checkers permit, and in part because in this case the user has explicitly declared their intent for the TypedDict to contain extra items, so it feels reasonable to account for that more strictly. This does mean that implicitly open typed dicts are not equivalent to extra_items=ReadOnly[object], which maybe in principle they would be -- but I think this is currently true in all type checkers. I think ty's compromise is reasonable, but I'm not totally convinced it's the best option, open to other resolutions here. But enforcing this rule on all implicitly open TypedDicts will have a significant impact on existing real-world code. |
Sorry, something went wrong.
|
For Pyrefly my current thought is that this would be off-by-default and enabled in strict mode. |
Sorry, something went wrong.
|
That seems reasonable. I'm not sure if behavior that we wouldn't turn on by default in our own type checkers should be encoded as a conformance suite requirement, though? It seems to me that a strict mode which requires safety in unpacking an open TypedDict should probably also require safety in unpacking a list/Sequence? |
Sorry, something went wrong.
|
So do you think that we should just merge #1960 and delete this section from the typing spec entirely? Or weaken the first assertion to "may" and "E?" |
Sorry, something went wrong.
|
I would probably land this with the first assertion weakened (and discussion of that in the spec also), but that's not a strong preference, it just matches what we decided to do in ty, so naturally it already makes sense to me :) Very open to other opinions here. |
Sorry, something went wrong.
…ypedDicts optional
|
Does it make sense to differentiate between implicitly open and explicitly open (closed=False), by making errors mandatory for the latter? |
Sorry, something went wrong.
It might. Currently in ty we only apply the extra strictness with extra_items=, not with closed=False. I think the appeal of this is that keeps closed itself boolean rather than tri-valued (that is, closed=False is the default and providing it explicitly makes no difference), but closed=False alone is not the same thing as explicitly providing an extra_items type -- the latter more explicitly says "I expect there to be extra items in this dict of a particular type". But all of this is a judgment call, there's no clear right or wrong answer. Or rather, the "right" answer in the abstract is probably the universal strictness you initially proposed, it's just a question of whether that's workable in the ecosystem. |
Sorry, something went wrong.
|
The current TypedDict spec says that closed=False means the TypedDict is open, i.e. consistent with the default. I think we should stick with the TypedDict spec chapter's clear division of TypedDicts into states, and allow unsound behavior only if the TypedDict is open (as defined by the spec; the spec change in this PR should link to the glossary definition for the term). In the abstract I'd prefer to treat open TypedDicts similar to ones with extra_items=ReadOnly[object], but I agree that's probably a bridge too far right now. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you!
Sorry, something went wrong.
There was a problem hiding this comment.
I think there are a couple details here in the spec (one backwards statement, one broken link) that should be addressed -- everything else here is up to you whether you want to include it in this PR. Otherwise this looks ready to me.
I'm glad we are clearing this up now, before wider adoption of closed/extra_items TypedDicts.
Sorry, something went wrong.
There was a problem hiding this comment.
I'm happy with this.
I guess since it touches the spec and the conformance suite, we need a Discuss thread, the one-week wait, and an issue for Typing Council sign-off?
Sorry, something went wrong.
Sorry, something went wrong.
|
Sorry for taking such a long time here. I have tried to play with this quite a bit in Zuban to get a feel for it and I'm favorable to merging this PR. There have been two potential cases that I would like to add to the spec (please let me know if you disagree): from typing import TypedDict
from typing_extensions import Unpack
class Person(TypedDict):
name: str
age: int
class Base:
def foo(self, **kwargs: Unpack[Person]) -> None: ...
class SubGood(Base):
def foo(self, *, name: str, age: int, extra: bool = False) -> None: ... # Should probably error here?!
This is currently not an error in Zuban/Mypy (test is called testUnpackKwargsOverrides in Mypy), but likely should be. I want to avoid type checkers using different logic for normal callable subtyping and inheritance checking. Another case of callable subtyping that I looked at was this one that might be worth to add: from typing import Never, Unpack, TypedDict
class TDBase(TypedDict):
x: int
class TD(TDBase, total=False):
y: int
def a1(**kwargs: Unpack[TDBase]) -> None: .
def a2(**kwargs: Unpack[TD]) -> None: ...
if bool():
a1 = a2 # (1) This should error
else:
a2 = a1 # (2)
Before my changes to think of non-closed TypedDicts in Unpacks as essentially having extra_items=object, (2) had an error, now it's the other way around (1) errors. This is definitely better, it was buggy before. It feels like this isn't really part of the tests yet and I think it's central to this view of TypedDicts. (I'm aware that this might not be an issue in other type checker, because it just widens the type. But you can also create this case with Protocols like a1: <some protocol that mimics a1> = a2) I'm also open to the make some of the # E? to # E, but I don't have a strong opinion here, especially after Carl's discovery that the impact is not as bad. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Previously, this section of the spec was not exercised at all in the conformance tests, and also was not implemented by any type checker.
This PR updates the spec to account for closed & extra_items TypedDicts, and adds conformance tests.
There are 3 asserted errors:
This would supersede #1960, which proposed that we delete the section of the spec entirely.
@rchen152's comment in https://discuss.python.org/t/typing-spec-inconsistency-for-unpacking-typed-dict-kwargs/79640 favors deletion.