Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.1k
Add basic support for recursive TypeVar defaults (PEP 696)#16878
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This comment has been minimized.
This comment has been minimized.
| self.recursive_tvar_guard[tvar_id]=None | ||
| repl=repl.accept(self) | ||
| ifisinstance(repl,TypeVarType): | ||
| repl.default=repl.default.accept(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Shouldn't we.copy_modified() here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That doesn't work, unfortunately. This here is used to link nested TypeVarTypes so if it's changed / evaluated in one location the other stays in sync.
An example from the tests
T1=TypeVar("T1",default=str)T2=TypeVar("T2",default=T1)classClassD1(Generic[T1,T2]): ...k=ClassD1()reveal_type(k)# should be `ClassD1[str, str]`
For the first pass the type variables are as follows
T1`1=strT2`2 = T1`1=str
The section here now replaces the default forT2 with the tbd of the default ofT1:
T1`1=strT2`2 = <result of T1`1>
--
With.copy_modified it would be a copy of theT1'1 thus when it's evaluated it's not used forT2. The test would still return
ClassD1[str,T1`1=str]
At least that's how I understand it 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looked at it more closely. It's basically as said above. In particular, it's called infreshend_function_type_vars. We enter this function with linked TypeVars
callee.variables[0]==callee.variables[1].default# True
Then create new ones for each and link them again inexpand_type, so that
tvs[0]==tvs[1].default# True# and by extensionfresh.variables[0]==fresh.variables[1].default# True
Lines 119 to 130 in517f5ae
| deffreshen_function_type_vars(callee:F)->F: | |
| """Substitute fresh type variables for generic function type variables.""" | |
| ifisinstance(callee,CallableType): | |
| ifnotcallee.is_generic(): | |
| returncast(F,callee) | |
| tvs= [] | |
| tvmap:dict[TypeVarId,Type]= {} | |
| forvincallee.variables: | |
| tv=v.new_unification_variable(v) | |
| tvs.append(tv) | |
| tvmap[v.id]=tv | |
| fresh=expand_type(callee,tvmap).copy_modified(variables=tvs) |
mypy/semanal.py Outdated
| forname,tvar_exprindeclared_tvars: | ||
| tvar_expr_default=tvar_expr.default | ||
| ifisinstance(tvar_expr_default,UnboundType): | ||
| # Assumption here is that the names cannot be duplicated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Unfortunately that's not a safe assumption
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do you have an example I could try? Mypy will already complain about this here and reject the secondTypeVar call.
T1=TypeVar("T1",default=int)T1=TypeVar("T1",default=str)# error
error: Cannot redefine "T1" as a type variableerror: Invalid assignment targeterror: "int" not callableThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think you have to import the other TypeVar from a different module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That results in the same error message. The secondTypeVar call would just be ignored. The logic here would still as it only compares the actual TypeVar nametype_var_name = fullname.rpartition(".")[2]. Not the completefullname.
E.g. the comparison is withT1 not__main__.T1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I thought something like this might fail:
[case testTypeVarDefaultsMultipleFiles]from typing import Generic, TypeVarfrom file2 import T as T2T = TypeVar('T')class Gen(Generic[T, T2]): passdef func(a: Gen, b: Gen[str], c: Gen[str, str]) -> None: reveal_type(a) # N: Revealed type is "__main__.Gen[Any, builtins.int]" reveal_type(b) # N: Revealed type is "__main__.Gen[builtins.str, builtins.int]" reveal_type(c) # N: Revealed type is "__main__.Gen[builtins.str, builtins.str]"[file file2.py]from typing import TypeVarT = TypeVar('T', default=int)But this passes as expected, and I couldn't find a variation that fails. So I suppose this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This variation fails:
[case testTypeVarDefaultsMultipleFiles]from typing import Generic, TypeVarfrom file2 import T as T2T = TypeVar('T', default=T2)class Gen(Generic[T2, T]): passdef func(a: Gen, b: Gen[str], c: Gen[str, float]) -> None: reveal_type(a) # N: Revealed type is "__main__.Gen[builtins.int, builtins.int]" reveal_type(b) # N: Revealed type is "__main__.Gen[builtins.str, builtins.str]" reveal_type(c) # N: Revealed type is "__main__.Gen[builtins.str, builtins.float]"[file file2.py]from typing import TypeVarT = TypeVar('T', default=int)Expected: main:11: note: Revealed type is "__main__.Gen[builtins.int, builtins.int]" (diff) main:12: note: Revealed type is "__main__.Gen[builtins.str, builtins.str]" (diff) main:13: note: Revealed type is "__main__.Gen[builtins.str, builtins.float]"Actual: main:11: note: Revealed type is "__main__.Gen[builtins.int, T2?]" (diff) main:12: note: Revealed type is "__main__.Gen[builtins.str, T2?]" (diff) main:13: note: Revealed type is "__main__.Gen[builtins.str, builtins.float]"Not code I'd expect anyone to actually write, but it does seem like it should be legal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for the test, that helped! Tbh I could also have though about this earlier but it's actually quite simple to add a name lookup here 😅
Diff frommypy_primer, showing the effect of this PR on open source code: steam.py (https://github.com/Gobot1234/steam.py)+ steam/ext/commands/commands.py:851: error: Overloaded function implementation does not accept all possible arguments of signature 3 [misc]+ steam/ext/commands/commands.py:851: error: Overloaded function implementation cannot produce return type of signature 3 [misc] |
Ref:#14851