-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Apply generic class fix also to non-callable types #8030
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
Changes from 3 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 |
---|---|---|
|
@@ -764,8 +764,8 @@ def analyze_class_attribute_access(itype: Instance, | |
t = get_proper_type(t) | ||
if isinstance(t, FunctionLike) and is_classmethod: | ||
t = check_self_arg(t, mx.self_type, False, mx.context, name, mx.msg) | ||
result = add_class_tvars(t, itype, isuper, is_classmethod, | ||
mx.builtin_type, mx.self_type, original_vars=original_vars) | ||
result = add_class_tvars(t, isuper, is_classmethod, | ||
mx.self_type, original_vars=original_vars) | ||
if not mx.is_lvalue: | ||
result = analyze_descriptor_access(mx.original_type, result, mx.builtin_type, | ||
mx.msg, mx.context, chk=mx.chk) | ||
|
@@ -808,9 +808,8 @@ def analyze_class_attribute_access(itype: Instance, | |
return typ | ||
|
||
|
||
def add_class_tvars(t: ProperType, itype: Instance, isuper: Optional[Instance], | ||
def add_class_tvars(t: ProperType, isuper: Optional[Instance], | ||
is_classmethod: bool, | ||
builtin_type: Callable[[str], Instance], | ||
original_type: Type, | ||
original_vars: Optional[List[TypeVarDef]] = None) -> Type: | ||
"""Instantiate type variables during analyze_class_attribute_access, | ||
|
@@ -821,12 +820,18 @@ class A(Generic[T]): | |
def foo(cls: Type[Q]) -> Tuple[T, Q]: ... | ||
|
||
class B(A[str]): pass | ||
|
||
B.foo() | ||
|
||
original_type is the value of the type B in the expression B.foo() or the corresponding | ||
component in case if a union (this is used to bind the self-types); original_vars are type | ||
variables of the class callable on which the method was accessed. | ||
Args: | ||
t: Declared type of the method (or property); | ||
isuper: Current instance mapped to the superclass where method was defined, this | ||
is usually done by map_instance_to_supertype(); | ||
is_classmethod: True if this method is decorated with @classmethod; | ||
original_type: The value of the type B in the expression B.foo() or the corresponding | ||
component in case of a union (this is used to bind the self-types); | ||
original_vars: Type variables of the class callable on which the method was accessed. | ||
Returns: | ||
Expanded method type with added type variables (when needed). | ||
""" | ||
# TODO: verify consistency between Q and T | ||
|
||
|
@@ -851,10 +856,12 @@ class B(A[str]): pass | |
t = cast(CallableType, expand_type_by_instance(t, isuper)) | ||
return t.copy_modified(variables=tvars + t.variables) | ||
elif isinstance(t, Overloaded): | ||
return Overloaded([cast(CallableType, add_class_tvars(item, itype, isuper, is_classmethod, | ||
builtin_type, original_type, | ||
return Overloaded([cast(CallableType, add_class_tvars(item, isuper, | ||
is_classmethod, original_type, | ||
original_vars=original_vars)) | ||
for item in t.items()]) | ||
if isuper is not None: | ||
t = cast(ProperType, expand_type_by_instance(t, isuper)) | ||
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. 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 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 actually have trouble understanding how this can not work :-) My guess is confusion comes from missing docstring entry for 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. 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 commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I was confused about |
||
return t | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2337,3 +2337,63 @@ 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. |
||
class F(G[List[int]]): ... | ||
|
||
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*]' | ||
|
||
xf: F | ||
yf: Type[F] | ||
reveal_type(xf.test) # N: Revealed type is 'builtins.list*[builtins.list*[builtins.int]]' | ||
reveal_type(yf.test) # N: Revealed type is 'builtins.list*[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.
Style nit: We don't use
;
after each description of an argument in docstrings.