classmethod produces errors due to TypeVar

[Dreamsorcerer]

A minimal test case:

[case testClassMethod]
from typing import Generic, TypeVar

T = TypeVar("T", int, None)

class A(Generic[T]):
    @classmethod
    def func(cls, arg: int):
        return cls(arg)

    def __init__(self, arg: T):
        self.attr = arg

[builtins fixtures/classmethod.pyi]

This gives an error complaining that cls(arg) expects None, due to it expanding the TypeVar on the class object.

Expected behaviour is that no error is produced, and that the type of cls does not have the expanded TypeVar (i.e. it should receive the same type that would be seen on a separate line of code like: a = A(5).

One catch to changing this behaviour is correct handling when subclassing with a defined type: #9456 (comment)

I've had a go at this, but was unable to figure out how to fix it. This is what I've figured out though:
https://github.com/python/mypy/blob/master/mypy/semanal.py#L641
Within prepare_method_signature(), cls gets changed from Any to the class type.
This is done by calling fill_typevars(), which results in the type including T, rather than just remaining as a TypeInfo (which appears to be the type in an example such as a = A(5)).
I've tried just removing the fill_typevars(), but then it runs into an unimplemented RuntimeError.

Because it includes T at this point, when it later tries to typecheck the code, it will check the class method twice, once as A[int] and again as A[None], which obviously throws the error we see.

So, it seems like there needs to be some way to stop class methods getting the TypeVars filled out on the cls argument.

[Dreamsorcerer]

@gvanrossum I'm not sure I follow the reasoning on the cls(day, day). My assumption is that this would be the equivalent of running TimePeriod(date1, date2), where I am expecting the TypeVar to be defined by the date objects I'm passing in. i.e. The return type for both would be inferred as TimePeriod[date, date].
The fact that the error states that it expected None makes me think that it's the same error as the operand one. i.e. Mypy thinks these TypeVars should always be None.

[gvanrossum]

I just had a thought. IIRC for a "type var with value restriction" (like you have here) mypy actually tries to type-check the entire class/function twice (or more if there are more than 2 values). This seems like two different instances of a problem with that (I think the error on cls(day, day) is unrelated to the error about and, though they both look related to this phenomenon.

Alas, I can't help you much more than that, it will require some serious trawling through the source code to figure this out. Thanks for the report though!

[vbarbaresi]

I dug into the code and found that it comes from here:

mypy/mypy/checker.py

Lines 1400 to 1409 in eb50379

	# Make a copy of the function to check for each combination of
	# value restricted type variables. (Except when running mypyc,
	# where we need one canonical version of the function.)
	if subst and not self.options.mypyc:
	result = [] # type: List[Tuple[FuncItem, CallableType]]
	for substitutions in itertools.product(*subst):
	mapping = dict(substitutions)
	expanded = cast(CallableType, expand_type(typ, mapping))
	result.append((expand_func(defn, mapping), expanded))
	return result

It checks for each possible combination, so 4 type checks with these substitutions:

{1: datetime.date, 2: datetime.date}
{1: datetime.date, 2: None}
{1: None, 2: datetime.date}
{1: None, 2: None}

I'm not familiar with TypeVars so I'm not sure how to fix this. Would it make sense to check on an UnionType of the 2 constraints? I suppose it wasn't written like that for a reason though.

[Dreamsorcerer]

I think the behaviour looks wrong for the classmethod, where we're passing the variable in, so it should be valid if it's of any subtype.

For the init method, it sounds reasonable to test it with each combination of inputs for any errors. However, it should only raise an unreachable error if that error occurs in every case.

[Dreamsorcerer]

I've just updated the original description to focus on the remaining classmethod issue.
I've also included some debugging information, as I tried to figure out how to fix it, but didn't manage to make it as far as a solution.
Hopefully that's enough information for someone who knows what they are doing to put a fix in place easily.

[DetachHead]

i think mypy is correct here. at first glance it looks like it's bounding the generic when it shouldn't, but it's possible for the generic to already have a concrete type by the time the classmethod is called. for example:

from __future__ import annotations

from typing import Generic, TypeVar

T = TypeVar("T", int, str)


class A(Generic[T]):
    @classmethod
    def func(cls, arg: int) -> A[T]:
        return cls(arg)

    def __init__(self, arg: T):
        self.attr: T = arg


asdf: A[str] = A.func(1)
reveal_type(asdf) # Revealed type is "__main__.A[builtins.str]"

[Dreamsorcerer]

Not sure I understand your argument, where is the concrete type for the class?

In your example, you call A.func(1), this will return an instance of type A[int].
You've then assigned it to a variable with type A[str], which should result in a mypy error.
The return type of that classmethod should also be A[int], not A[T].

[DetachHead]

i'm saying that the generic could have a concrete type here, and that mypy is right to complain, though that example i gave probably isn't very clear. what about this one:

from typing import Generic, TypeVar

T = TypeVar("T", int, str)


class A(Generic[T]):
    class_attr: T

    @classmethod
    def set_attr(cls, arg: int) -> None:
        # mypy is right to complain here
        cls.class_attr = arg # error: Incompatible types in assignment (expression has type "int", variable has type "str")  [assignment]


B = A[str]

B.set_attr(1)

value = B().class_attr

# if it didn't complain, this would happen:
reveal_type(value) # note: Revealed type is "builtins.str*"
print(value) # 1

(though i think the error message itself is bad, see #11536)

anyway, looks like the solution is to just type the cls argument:

class A(Generic[T]):
    class_attr: T

    @classmethod
    def set_attr(cls: type[A[int]], arg: int) -> None:
        # error is gone
        cls.class_attr = arg

B = A[str]

#now we correcltly get an error here instead
B.set_attr(1)

so for your original example:

class A(Generic[T]):
    @classmethod
    def func(cls: type[A[int]], arg: int):
        # no more error
        return cls(arg)

    def __init__(self, arg: T):
        self.attr = arg

[Dreamsorcerer]

I'm afraid there is no such thing as a concrete class type as you've just described, the example you managed to pass in mypy should in fact have given errors as it is completely broken. Try expanding your example to:

class A(Generic[T]):
    class_attr: T

    @classmethod
    def set_attr(cls: Type["A[T]"], arg: T) -> None:
        cls.class_attr = arg

B = A[int]
C = A[str]

B.set_attr(1)
C.set_attr("foo")

value = B().class_attr

reveal_type(value) # note: Revealed type is "builtins.int*"
print(value) # "foo"

There is only 1 concrete class, A, and your code has set the single class attribute shared across all types. A class is not generic, the instance is (although maybe you can do something like your proposal with metaclasses, but that's outside the scope of this issue).

Therefore, even if you have done B = A[int], this is just a type alias and should only be used for type annotations, at runtime it is ultimately the same as A and therefore any class methods should still consider the cls parameter to be the generic A class, not a concrete type.

[DetachHead]

i see your point, though imo this just scratches the surface of a huge hole in the type system (see #11537 and #11538) so i don't think removing checks like this is the best solution

[Dreamsorcerer]

i see your point, though imo this just scratches the surface of a huge hole in the type system (see #11537 and #11538) so i don't think removing checks like this is the best solution

I'm not suggesting removing any valid checks, I'm suggesting that classmethods should be checked with the first argument being a generic type, because that's exactly what it is, instead of checking it for each concrete type, which can never actually happen.

e.g. This should also work without problems:

class A(Generic[T]):
    @classmethod
    def func(cls, arg: int) -> A[int]:
        return cls(arg)

    def __init__(self, arg: T):
        self.attr = arg

a: A[str] = A("foo")
b: A[int] = a.func(1)

The fact that a is A[str] has no bearing on the class method, cls is still A[Generic[T]] and the method can still return a type different from the instance type. It makes no sense that it checks cls(1) as expecting both an int and str at the same time.

[Dreamsorcerer]

And if I add your 'fix' of specifying the cls type as Type[A[int]], then that previous example still fails, as it says the self argument is Type[A[str]], which is also incorrect.

[KotlinIsland]

@Dreamsorcerer Don't forget about polymorphic cls

This error is entirely correct, just perhaps in your example it's not being abused.

from __future__ import annotations
from typing import Generic, TypeVar

T = TypeVar("T", int, str)

class A(Generic[T]):
    @classmethod
    def func(cls, arg: int) -> A[int]:
        return cls(arg)  # error: Incompatible return value type (got "A[str]", expected "A[int]")

    def foo(self, arg: T) -> None:
        pass

    def __init__(self, arg: T):
        self.attr: T = arg

class B(A[str]):
    def foo(self, arg: str) -> None:
        print(arg.upper())

b1: A[str] = B("foo")
b2: A[int] = b1.func(1)

b2.foo(1)  # runtime error: AttributeError: 'int' object has no attribute 'upper'

for example

def foo(a: int) -> str:
    return str(a)
    
foo("AMONGUS")  # error: Argument 1 to "foo" has incompatible type "str"; expected "int"

Why would this complain if it's not going to fail at runtime?

This issue is that even though your specific example will work correctly at runtime, it's not typesafe and will most certainly lead to type errors down the line.

[KotlinIsland]

@Dreamsorcerer

There is only 1 concrete class, A

Therefore, even if you have done B = A[int], this is just a type alias and should only be used for type annotations, at runtime it is ultimately the same as A and therefore any class methods should still consider the cls parameter to be the generic A class, not a concrete type.

Generally a concrete type refers to any type that has no unspecified type parameters. By that definition, A[int] is considered a concrete type(compared to the non-concrete A) and can also be correctly used outside of annotations, even if it it is partially erased.

from __future__ import annotations
from typing import Generic, TypeVar, ClassVar

T = TypeVar("T")


class A(Generic[T]):
    @classmethod
    def foo(cls, a: T) -> None:
        ...
    @classmethod
    def bar(cls: type[A[int]]) -> None:
        ...

Int = A[int]  # concrete type
Str = A[str] # concrete type
A_Alias = A  # non-concrete type

A_Alias.foo(1)  # correct usage not as an annotation
Str.foo("AMONGUS")  # correct usage not as an annotation

Int.bar()  # correct usage not as an annotation

[Dreamsorcerer]

Don't forget about polymorphic cls

Yep, that certainly adds some complication when inheriting from a defined type. I don't know if maybe mypy can detect that such a classmethod cannot be used in a subclass or something, but sounds a bit messy either way.

Generally a concrete type refers to any type that has no unspecified type parameters. By that definition, A[int] is considered a concrete type(compared to the non-concrete A) and can also be correctly used outside of annotations, even if it it is partially erased.

I don't see the usecase for your last example. Why does it matter what type you assigned as the class for that classmethod? If you stick with the idea that cls is always generic, then the type is defined by the parameter to the classmethod.

@classmethod
def foo(cls, a: T):
    cls  # Must be: Type[A[T]]

Int.foo("bar")  # cls is Type[A[str]]

What practical purpose is there to have cls predefined as A[int] in a classmethod like this (given that we've already shown that generic class attributes are not a thing)?

[KotlinIsland]

such a classmethod cannot be used in a subclass or something

That would break LSP, mypy is correct to point out this invalid code.

I don't see the usecase for your last example

Thats how the type system works, it doesn't matter if you can't think of a use case for it.

If you stick with the idea that cls is always generic, then the type is defined by the parameter to the classmethod.

You can't say that cls is always the same type as the parameter because it's polymorphic, it could be bound with any arbitrary type, as the example that I provided.

[KotlinIsland]

If you made the class final then cls could no longer be polymorphic.

from typing import Generic, TypeVar, final

T = TypeVar("T")


@final
class A(Generic[T]):
    @classmethod
    def func(cls, arg: int) -> A[int]:
        return cls(arg)  # error: Argument 1 to "A" has incompatible type "int"; expected "T"

    def __init__(self, arg: T):
        self.attr: T = arg

This just looks very inconsistent though, and special casing this behavior would just increase complexity unnecessarily and cause more confusion on the users end in my opinion.

And at that point, why not just do:

    @classmethod
    def func(cls, arg: int) -> A[int]:
        return A(arg)  # no error

I think this issue should be closed as I don't see anything wrong here.

[erictraut]

I agree that mypy is doing the right thing in this case. The type of the cls parameter in a classmethod is necessarily type[Self], which means that it adopts the type arguments of the class. It can't be generic and take on some other type arguments within the classmethod. That would break the type system. I agree that this issue should be closed.