| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
I'm looking at what (I think) is the corresponding section of the conformance suite, and the function has an extra parameter corresponding to the extra key in TD2: class TD1(TypedDict):
v1: Required[int]
v2: NotRequired[str]
class TD2(TD1):
v3: Required[str]
def func2(v3: str, **kwargs: Unpack[TD1]) -> None: ...
def func3() -> None:
func2(**td2) # OK
If you delete the v3 parameter from func2, mypy does error (https://mypy-play.net/?gist=4d52cf2dbdaf2e4ecee1e2b54bc3d138), although pyright and pyrefly don't. So the conformance suite doesn't seem inconsistent to me, although pyright and pyrefly's implementations are (and arguably don't conform to the spirit of what's being tested). With that said, I'd be fine with modifying the conformance test to also allow a consistent **kwargs: object interpretation. |
Sorry, something went wrong.
There was a problem hiding this comment.
We can have a discussion about which of these interpretations should be preferred, but given the lack of clarity about this in the spec, I think for now a conformant implementation should be allowed to choose either interpretation and implement it consistently.
I agree, though I personally prefer the version where this errors even if - from a theoretical perspective - this should pass. The reason for this is that most people are probably using Unpack[TypedDict] to reuse some kwargs and for that it will catch annoying errors, because people are going to forget that they should have used extra_items=Never or closed=True.
I would just want to avoid making type checkers non-conformant for being more "strict" here.
Sorry, something went wrong.
|
@rchen152 Sorry I wasn't clearer -- I wasn't saying that the conformance suite contains that example, just that pyright and pyrefly do allow it. What the conformance suite does contain is the requirement (added in #1918) that a callable that doesn't accept *kwargs is not assignable to a callable that accepts **kwargs: Unpack[TD] (this assertion). The prose justification for that requirement that was added to the spec is the case of unpacking **td2 to **kwargs: Unpack[TD], even though that call is not tested in the conformance suite. So this is the contradiction I see in the current conformance suite: it simultaneously requires that calls to f act as if f does not accept **kwargs, and that assignability of f to other callable types act as if f does accept **kwargs. I don't think we should specify or require this contradiction. Relatedly, I think that the spec now has a lot of complex cases and language around assignability of callables using **kwargs: Unpack[TD]. I believe that all of that can and should be replaced by a much simpler specification that outlines how to transform **kwargs: Unpack[TD] into a "normal" signature, and then simply specify that all assignability (and call evaluation) rules follow from that transformation. @davidhalter Yes, I think that's defensible. I think that a type-checker which consistently implemented that approach should not allow the **td2 call either, nor should it implement the assignability error in the func7 assertion I linked above. Because that interpretation says that the signature of f is simply (*, v1: int), in which case its callable type should be equivalent to the callable type of a function defined explicitly as def _(*, v1: int). Thanks both for the reviews! |
Sorry, something went wrong.
There was a problem hiding this comment.
Ah, got it. Thanks for the clarification!
Sorry, something went wrong.
There was a problem hiding this comment.
I think this is a step in the right direction. In the long term I feel we should consistently treat open TypedDicts as if they can have arbitrary extra keys (because they can), which means the call at issue here should be allowed by type checkers.
But I think it's OK to wait with that until PEP 728 is more widely used and supported.
Closed TypedDicts are arguably more intuitive and better behaved in many ways; perhaps they should have been the default. It's too late for that though.
Sorry, something went wrong.
* main: Docs: Fix typo and grammar in type narrowing guide (python#2271) Update ty results for v0.0.35 (python#2282) Add pycroscope to the conformance suite (python#2278) Bump type checkers (python#2279) Update ty results for v0.0.33 (python#2275) conformance: allow consistent treatment of kwargs: Unpack[TD] (python#2272) bump mypy and pyright (python#2274) fix pyright version parsing (python#2273) Allow type checkers to ignore specific error codes (python#2153) Update pyrefly conformance results to v0.62.0 (python#2269) add ty to scripts/bump_type_checkers.py (python#2268)
| Back | FazBrowse Home | New Git URL |
The conformance suite currently asserts this (simplified for clarity):
This implies that the signature of f de-sugars to f(*, v1: int). But the current conformance suite simultaneously also requires that callable assignability behave as if it de-sugars to f(*, v1: int, **kwargs: object). That addition to the conformance suite was justified by this call, which is allowed by pyright and pyrefly:
It seems reasonable to allow this call, because TD2 is a subtype of TD1 and assignable to it, so **td2 should therefore be acceptable for **kwargs: Unpack[TD1]. But if this call (which definitely provides a keyword argument v2) is allowed, then we should also allow the above call that explicitly provides v2.
I think we could defensibly pick either of the interpretations of the signature of f (with or without **kwargs: object), but type checkers should be allowed to pick a consistent interpretation, not be required to implement an inconsistent hybrid behavior.
The interpretation with **kwargs: object is IMO more consistent and better supported by the spec. The spec says that if TD has extra_items, then arbitrary additional keyword arguments of that type should be accepted. And it also says that a TypedDict without an explicit closed or extra_items parameter usually behaves as if it has extra_items of type ReadOnly[object].
The interpretation without **kwargs: object would effectively be saying that **kwargs: Unpack[TD] is one of these special cases (along with TypedDict constructor calls) where an unmarked TypedDict is (inconsistent with its "open" semantics) not treated as having extra_items of type ReadOnly[object].
We can have a discussion about which of these interpretations should be preferred, but given the lack of clarity about this in the spec, I think for now a conformant implementation should be allowed to choose either interpretation and implement it consistently.