-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Apply generic class fix also to non-callable types #8030
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2337,3 +2337,57 @@ class Test(): | |
reveal_type(MakeTwoAppliedSubAbstract()(2)) # N: Revealed type is '__main__.TwoTypes[builtins.str, builtins.int*]' | ||
reveal_type(MakeTwoGenericSubAbstract[str]()('foo')) # N: Revealed type is '__main__.TwoTypes[builtins.str, builtins.str*]' | ||
reveal_type(MakeTwoGenericSubAbstract[str]()(2)) # N: Revealed type is '__main__.TwoTypes[builtins.str, builtins.int*]' | ||
|
||
[case testGenericClassPropertyBound] | ||
from typing import Generic, TypeVar, Callable, Type, List, Dict | ||
|
||
T = TypeVar('T') | ||
U = TypeVar('U') | ||
|
||
def classproperty(f: Callable[..., U]) -> U: ... | ||
|
||
class C(Generic[T]): | ||
@classproperty | ||
def test(self) -> T: ... | ||
|
||
class D(C[str]): ... | ||
class E1(C[T], Generic[T, U]): ... | ||
class E2(C[U], Generic[T, U]): ... | ||
class G(C[List[T]]): ... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about testing if there are two levels of generic subclasses, such as What if the derived class adds a type variable? There may be two interesting cases -- 1) derived class adds a type variable that becomes the first type variable 2) derived class adds a type variable the becomes the second type variable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can add few more tests, but essentially this will be testing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the extra tests don't seem useful. |
||
|
||
x: C[int] | ||
y: Type[C[int]] | ||
reveal_type(x.test) # N: Revealed type is 'builtins.int*' | ||
reveal_type(y.test) # N: Revealed type is 'builtins.int*' | ||
|
||
xd: D | ||
yd: Type[D] | ||
reveal_type(xd.test) # N: Revealed type is 'builtins.str*' | ||
reveal_type(yd.test) # N: Revealed type is 'builtins.str*' | ||
|
||
ye1: Type[E1[int, str]] | ||
ye2: Type[E2[int, str]] | ||
reveal_type(ye1.test) # N: Revealed type is 'builtins.int*' | ||
reveal_type(ye2.test) # N: Revealed type is 'builtins.str*' | ||
|
||
xg: G[int] | ||
yg: Type[G[int]] | ||
reveal_type(xg.test) # N: Revealed type is 'builtins.list*[builtins.int*]' | ||
reveal_type(yg.test) # N: Revealed type is 'builtins.list*[builtins.int*]' | ||
|
||
class Sup: | ||
attr: int | ||
S = TypeVar('S', bound=Sup) | ||
|
||
def func(tp: Type[C[S]]) -> S: | ||
reveal_type(tp.test.attr) # N: Revealed type is 'builtins.int' | ||
|
||
reg: Dict[S, G[S]] | ||
reveal_type(reg[tp.test]) # N: Revealed type is '__main__.G*[S`-1]' | ||
reveal_type(reg[tp.test].test) # N: Revealed type is 'builtins.list*[S`-1]' | ||
|
||
if bool(): | ||
return tp.test | ||
else: | ||
return reg[tp.test].test[0] | ||
[builtins fixtures/dict.pyi] |
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.
It would be nice to have a comment here, or maybe add a case to the above comment where the type is not callable.
Also, I have trouble convincing myself how we can be sure that the type arguments from
isuper
can always be applied tot
. Maybe explain this is briefly (in the above comment perhaps, since this doesn't seem specific to this case).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 actually have trouble understanding how this can not work :-) My guess is confusion comes from missing docstring entry for
isuper
, which is current instance mapped to the method definition superclass. I was also thinking about an assert, but this would be probably too much.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.
Btw, while adding docstring items I noticed there are two unused arguments in this function.
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.
Yeah, I was confused about
isuper
. Now it all makes sense.