Skip to content

Generic class atributes aren't fully instantiated #2878

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

Closed
miedzinski opened this issue Feb 17, 2017 · 4 comments
Closed

Generic class atributes aren't fully instantiated #2878

miedzinski opened this issue Feb 17, 2017 · 4 comments

Comments

@miedzinski
Copy link
Contributor

T = TypeVar('T')
class A(Generic[T]):
    def f(self, x: T) -> None: ...
class B(Generic[T]):
    x = None  # type: A[T]

B[int].x.f(0)  # E: Argument 1 to "f" of "A" has incompatible type "int"; expected "T"

Discovered while reviewing #2873.

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 22, 2017

I don't think that the current semantics actually support that. Consider this:

from typing import TypeVar, Generic

T = TypeVar('T')

class A(Generic[T]):
    x = None  # type: T

A[int].x = 1
A[str].x = 'a'
print(A[int].x)  # 1
print(A[str].x)  # a

a = A()  # type: A[int]
print(a.x)  # None
print(type(a).x)  # None
print(A[int]().x)  # None

The value of x depends on how we access it, which seems pretty bad. This looks like a problem in typing -- @ilevkivskyi do you agree?

Mypy could work around this even with the current semantics by rejecting code like A[int].x -- the only use supported for A[int] outside a type context would be A[int](...) where we don't have this problem.

Also, attributes that use class type variables should only be accessible through an instance, not the class. So A().x would be okay but A.x would be rejected, since the value of the type variable is conceptually bound to an instance, not the type object. There's effectively only a single type object A (though the implementation sometimes behaves like this is not the case).

@ilevkivskyi
Copy link
Member

The value of x depends on how we access it, which seems pretty bad. This looks like a problem in typing -- @ilevkivskyi do you agree?

I agree this looks strange. One possible solution (first that comes to my mind) is to tweak descriptor __dict__ of GenericMeta so that all assignments and look-ups on subscripted classes (A[int] etc ) will be transferred to the original class object A. But I am not comfortable making this change without discussion with Guido, since this is a change in "user facing API".

Also, attributes that use class type variables should only be accessible through an instance, not the class. So A().x would be okay but A.x would be rejected, since the value of the type variable is conceptually bound to an instance, not the type object. There's effectively only a single type object A (though the implementation sometimes behaves like this is not the case).

I think I agree. Effectively this means that in the proposed implementation of ClassVar #2873 we need to prohibit generic class variables.

But what should we do with look-up of instance variables (those not wrapped in ClassVar) on classes in case if these variables are generic? Just leave it as it is, or set all type variables to Any on such look-up, or something else? (I think just prohibiting this always is not a good solution).

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 24, 2017

But what should we do with look-up of instance variables (those not wrapped in ClassVar) on classes in case if these variables are generic? Just leave it as it is, or set all type variables to Any on such look-up, or something else? (I think just prohibiting this always is not a good solution).

Prohibiting it seems to be an easy and safe solution. If we give it any normal type, this would make it possible to assign to the default:

A.x = something

This would break x used as an instance attribute, since the value can't be compatible with all possible type variable values (unless the value is None and we aren't using --strict-optional, but that doesn't seem very useful).

However, if we make it a read-only attribute when accessed through the class, we could sometimes use the upper bound type of the type variable, but that wouldn't work for types list List[T] that are invariant -- we'd need an existential type which we don't have.

I can't think of any use reasonable case where it would be important to access the default value of a instance variable that has a generic type variables in it's type through a type object. If you can come up with an example, it could be easier to see what we should do, other than rejecting it.

@ilevkivskyi
Copy link
Member

@JukkaL

I can't think of any use reasonable case where it would be important to access the default value of a instance variable that has a generic type variables in it's type through a type object. If you can come up with an example, it could be easier to see what we should do, other than rejecting it.

My main concern is that the user who are not aware of instance/class variable distinction will be confused if their perfectly working code is rejected. The problem is finding a good error message.
The best I can think now is something like:

class C(Generic[T]):
    x: T
y = C.x # E: Access to generic instance variables via class is ambiguous

(Btw the runtime issue in typing is now fixed)

@JukkaL JukkaL added bug mypy got something wrong priority-1-normal and removed needs discussion labels Mar 8, 2017
JukkaL pushed a commit that referenced this issue Mar 9, 2017
Implements ClassVar introduced in PEP 526. Resolves #2771, #2879.

* Support annotating class attributes with ClassVar[...].
* Prohibit ClassVar annotations outside class bodies (regular variable
  annotations, function and method signatures).
* Prohibit assignments to class variables accessed from instances (including
  assignments on self).
* Add checks for overriding class variables with instance variables and vice
  versa when subclassing.
* Prohibit ClassVars nested inside other types.
* Add is_classvar flag to Var.
* Add nesting_level attribute to TypeAnalyser and use it inside helper methods
  anal_type and anal_array. Move analyzing implicit TupleTypes from
  SemanticAnalyzer to TypeAnalyser.
* Analyze ClassVar[T] as T and bare ClassVar as Any.
* Prohibit generic ClassVars and accessing generic instance variables via class
  (see #2878 comments).
* Add types.get_type_vars helper, which returns all type variables present in
  analyzed type, similar to TypeAnalyser.get_type_var_names.
@JukkaL JukkaL closed this as completed Mar 9, 2017
pkch added a commit to pkch/peps that referenced this issue Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants