Skip to content
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

Type variables bound by a generic class make no sense in static or class methods. #3172

Open
sixolet opened this issue Apr 14, 2017 · 5 comments

Comments

@sixolet
Copy link
Collaborator

sixolet commented Apr 14, 2017

Here's a type variable in a normal method. This works fine:

from typing import TypeVar, Generic

T = TypeVar('T')
class A(Generic[T]):

    def f(self) -> None:
        x: T

However in a static method this doesn't make sense, since we don't have an instance of A to provide any parameterized type for T:

from typing import TypeVar, Generic

T = TypeVar('T')
class A(Generic[T]):
    @staticmethod
    def f() -> None:
        x: T  # Error we don't yet provide: Invalid type "__main__.T"

Nor does this make any sense:

from typing import TypeVar, Generic

T = TypeVar('T')
class A(Generic[T]):
    @classmethod
    def f(cls) -> None:
        x: T  # Error we don't yet provide: Invalid type "__main__.T"
@sixolet
Copy link
Collaborator Author

sixolet commented Apr 14, 2017

Note also that an assignment statement within a class outside a normal method should by all rights have the same problem, but people are already using those to make type declarations of instance variables, using a style reminiscent of PEP526 but punning them with class variables. I don't really know what to do about those. I think it'd be cleanest to forbid, and let people define instance variables using generics when they're actually using PEP526 syntax instead of faking it. That might not be practical.

@ilevkivskyi
Copy link
Member

@sixolet Note that variables declared in class body are always instance variables, class variables should be declared by ClassVar.

There was a discussion about generic class/instance attributes in #2878, and it was fixed in #2873, so that now mypy behaves like this:

class A(Generic[T]):
    x: T  # OK
    y: ClassVar[T]  # E: Invalid type: ClassVar cannot be generic
A.x  # E: Access to generic instance variables via class is ambiguous
A[int].x  # E: Access to generic instance variables via class is ambiguous
A[int]().x  # OK

Concerning type variables in static and class methods, I am not sure whether we should prohibit this. In any case, I think this is quite low priority.

@sixolet
Copy link
Collaborator Author

sixolet commented Apr 14, 2017

@ilevkivskyi Yes, that's correct for PEP526-type variables. However:

>>> class A:
... 	x = 3
... 
>>> A.x
3

When you use a simple assignment, rather than just a type declaration, it sure seems like the semantics of a class variable to me. You're right that it's much less dangerous, being that you can't access the variable without making a member expr on the class, and you get that beautiful error message when you do.

Anyway, whatever the priority, my sense of completeness makes me want to fix this for classmethods and staticmethods while I understand exactly how, so once #3113 is merged I'll make another small PR -- I think I know exactly what code to write. Unless you can find me a way where using a type variable bound by the generic makes sense outside an instance context.

@ilevkivskyi
Copy link
Member

When you use a simple assignment, rather than just a type declaration, it sure seems like the semantics of a class variable to me.

I think it is not right time to discuss this. PEP 526 already defines this as an "instance variable with default".

Unless you can find me a way where using a type variable bound by the generic makes sense outside an instance context.

I have no "practical" use case, but I could imagine something like:

class X(Generic[T]):
    attr: List[T]
    @classmethod
    def make_combined(cls, other: X[T]) -> X[T]:
        new_attr: List[T] = []
        instance = cls()
        new_attr.extend(other.attr)  # we want to make this safe by using 'T' above
        instance.attr = new_attr
        return instance

In any case, I don't think we can or should prohibit all meaningless/useless cases in mypy. What kind of errors you want to prevent by prohibiting this?

@sixolet
Copy link
Collaborator Author

sixolet commented Apr 17, 2017

In the case you give, T within the classmethod seems like it should be a parameter bound by the method, rather than the class, and your instance should be cls[T]() or something.

As for "prohibit all meaningless/useless cases" I am in full agreement about useless cases -- they don't hurt anything. I disagree about meaningless cases. I think all valid type annotations should have a meaning, however useless, and if you can't define the meaning, the annotation should not be valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants