Covariant type variables should be allowed in class method

[saulshanabrook]

Currently, covariant type variables are allowed in __init__ so that you can put create an immutable containers with something inside of it. See #2850.

It would be useful if covariant types are also allowed in class method that serve as alternative constructors. Currently mypy errors on this example, which is an extension of the other issues example:

from typing import Generic, TypeVar

T_co = TypeVar("T_co", covariant=True)


class C(Generic[T_co]):
    def __init__(self, x: T_co) -> None:
        ...

    @classmethod
    def custom_init(cls, x: T_co) -> "C[T_co]":
        # error: Cannot use a covariant type variable as a parameter
        return cls(x)

My use case is trying to define an immutable container protocol that has a create class method, which will initialize the container with some contents, like this:

T_cov = typing.TypeVar("T_cov", covariant=True)

class ListProtocol(typing_extensions.Protocol[T_cov]):
    @classmethod
    def create(cls, *items: T_cov) -> "ListProtocol[T_cov]":
        ...

    def getitem(self) -> T_cov:
        ...
    
    ...

[ilevkivskyi]

This proposal (add a @covariant_args decorator) appeared before in context of an old issue #1237. It appeared few times since then, so I think this may be a reasonable feature to support.

[JukkaL]

There is an easy workaround -- use a separate, non-covariant type variable:

from typing import Generic, TypeVar

T_co = TypeVar("T_co", covariant=True)
T = TypeVar("T")

class C(Generic[T_co]):
    def __init__(self, x: T_co) -> None:
        ...

    @classmethod
    def custom_init(cls, x: T) -> "C[T]":   # OK
        return cls(x)

We could update the error message to suggest this. Defining an extra type variable seems like busywork, though.

I don't think that @covariant_args is necessarily relevant here. Using a covariant type variable currently only has semantic significance when used as a class type variable. Allowing a covariant type variable to be used at function/method level wouldn't necessarily change the rules used for signature compatibility when overriding. One option would be to treat them as equivalent to invariant type variables.

[remdragon]

What's the status of this request?

I'm generating stubs for wxPython and running into the following situation:

class TextEntry: # from wx
    def GetValue ( self ) -> unicode: ...

class NumCtrl ( TextEntry ): # from wx.lib.masked.numctrl
    def GetValue ( self ) -> int: ... # error: Return type of "GetValue" incompatible with supertype "TextEntry"

At the moment I am considering the work-around of subclassing NumCtrl and using a differently named function to access GetValue() which I'll protect with a "type: ignore". I only use NumCtrl in a couple places so this is an acceptable solution this time, but I have a feeling I may run into this a few more times on this project, so I thought I'd ask about it here.

If there's a cleaner solution than subclassing and changing the names to protect the guilty, I'd love to hear it.

[ilevkivskyi]

@remdragon The code you posted is actually not type-safe, it clearly violates LSP and unlikely to be supported. There was a similar discussion about Qt, see #1237 that led to a proposal of "implementation only inheritance" see python/typing#241, but this whole topic got very little traction, so your best bet is to just use # type: ignore in definition of NumCtrl.

Also # type: ignore just silences the error, it doesn't erase the type of the method or anything, mypy will still think that NumCtrl().GetValue() is an int.

[remdragon]

Yes, I know it's not type safe. Unfortunately, there's no chance convincing wxPython to fix their API to be LSP compliant.

The work-around I came up with, which may bite me in other ways later, is to just not declare NumCtrl as a subclass of TextEntry. This may require me to add more entries than necessary, and I lose the ability to pass NumCtrl around as a TextEntry, but I believe these may be acceptable trade-offs this time.

There was only one other LSP conflict that I ran into, but I was able to resolve in a similar way.

At this point my wx.pyi is complete enough that I am catching bugs and only making minor additions to it to support this project, so I am satisfied. I only asked because it looked like you guys might have been leaning towards a way to selectively switch to an LSP alternative which would have been desirable here.

[ilevkivskyi]

Note that workaround proposed by @JukkaL above is actually not safe and now correctly produce an error, so we should find another solution.

[gre7g]

@remdragon Are your stubs for wxPython available? I need some and would love to share, please!

[remdragon]

sure, here you go. Royce Mitchell, IT Consultant ITAS Solutions [email protected] There are three hard problems in computer science: naming things, and off-by-one errors.

…

On Wed, Jun 10, 2020 at 4:52 PM Gre7g Luterman ***@***.***> wrote: @remdragon <https://github.com/remdragon> Are your stubs for wxPython available? I need some and would love to share, please! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6178 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4RVV6XDNOYXEDXORWFELLRV76A5ANCNFSM4GPIB3VQ> .

[gre7g]

@remdragon Sorry if I'm being a newb here, but here where? Is it in a branch? Do you have a link? What am I missing?

[remdragon]

I'm the noob. I tried to attach it but apparently github blocked it. I have created a public repo under my account with the files. Fair warning, there are certain parts of PEP8 that I passionately and violently object to... Royce Mitchell, IT Consultant ITAS Solutions [email protected] There are three hard problems in computer science: naming things, and off-by-one errors.

…

On Wed, Jun 10, 2020 at 6:07 PM Gre7g Luterman ***@***.***> wrote: @remdragon <https://github.com/remdragon> Sorry if I'm being a newb here, but here where? Is it in a branch? Do you have a link? What am I missing? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6178 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4RVV3FCH7TZQIJWN2ML3LRWAG2NANCNFSM4GPIB3VQ> .

[ktbarrett]

Another example for what's described in the OP.

class ImmutableList(Generic[T_co]):
    def concat(self, item: T_co) -> ImmutableList[T_co]: ...

Not sure where the justification for not allowing covariant TypeVars came from, but it's totally unnecessary. While variance on free variables makes no sense, not allowing the use of bound variables that are covariant or contravariant (but invariant is fine) is an unnecessary limitation.

I think a simple fix is to check to see if the TypeVar is bound before issuing the error message.

[ethanabrooks]

Another example from the section of the docs on precise typing of alternative constructors:

T = TypeVar('T')

class Base(Generic[T]):
    Q = TypeVar('Q', bound='Base[T]')

    def __init__(self, item: T) -> None:
        self.item = item

    @classmethod
    def make_pair(cls: Type[Q], item: T) -> tuple[Q, Q]:  # If T is covariant, you get error: Cannot use a covariant type variable as a parameter
        return cls(item), cls(item)

Suppose you want to make T covariant so that Base[A] is a subtype of Base[B] if A is a subtype of B. You can't do this because make_pair will complain about using a covariant type variable as a parameter.

[Prometheus3375]

I would like to address the issue with covariant types in arguments of methods of immutable containers.

class Sequence(Collection[_T_co], Reversible[_T_co], Generic[_T_co]):
    @overload
    @abstractmethod
    def __getitem__(self, i: int) -> _T_co: ...
    @overload
    @abstractmethod
    def __getitem__(self, s: slice) -> Sequence[_T_co]: ...
    # Mixin methods
    def index(self, value: Any, start: int = ..., stop: int = ...) -> int: ...
    def count(self, value: Any) -> int: ...
    def __contains__(self, x: object) -> bool: ...
    def __iter__(self) -> Iterator[_T_co]: ...
    def __reversed__(self) -> Iterator[_T_co]: ...

Implementation of index raises ValueError if given value is not found. If there is seq: Sequence[int], then seq.index('string') fails for sure. So, it is reasonable to restrict value to _T_co. But this causes mypy to report Cannot use a covariant type variable as a parameter.

[gsakkis]

I ran into this and while trying to find a workaround I discovered that although T_co cannot be a parameter, Optional[T_co] can. Why is the former considered unsafe but the latter isn't? Also since Optional[T_co] is actually Union[T_co, None] I tried Union[T_co, T_co] and surprisingly it works without an error too.

Playground url

[CraftSpider]

I appreciate the workaround posted. I just ran into this while working on a property-like decorator - the code looks kind of like the following (simplified):

class decorator(Generic[T, U_co]):
    def __init__(self, fget: Callable[[U_co], T]):
        ...

    @overload
    def __get__(
        self, value: U_co, owner: Optional[type[U_co]] = ...  # type: ignore[misc]
    ) -> Wrapped[T, U_co]:
        ...

    @overload
    def __get__(
        self, value: None, owner: Optional[type[U_co]] = ...
    ) -> decorator[T, U_co]:
        ...

    def __get__(
        self, value: Optional[U_co], owner: Optional[type[U_co]] = None
    ) -> Union[Wrapped[T, U_co], decorator[T, U_co]]:
        if value is None:
             return self
        return Wrapper(self.fget(value))

If U is instead invariant, subclasses cannot override the base-class definition, despite the fact that this is expected to work. And in this context, a covariant parameter is exactly the expected semantics. I'd love for mypy to support some way of confirming that you really are confident that this is what you want, and that the variance is correct.

[ktbarrett]

Some of the examples here have some issues.

@ethanabrooks's example doesn't need to be covariant AFAICT. The following works. Maybe I'm missing a use case that should work?

class C(Base[float]):
    ...

def wew(arg: tuple[Base[float], Base[float]]) -> None:
    ...

wew(C.make_pair(1.0))
wew(C.make_pair(1))

@Prometheus3375's example is not type safe.

def look_for_1_0(a: Sequence[float]) -> bool:
    try:
        a.index(1.0)
    except IndexError:
        return False
    else:
        return True
        
a: Sequence[int]
look_for_1_0(a)  # passing a Sequence[int] should fail here as 1.0 can't be in a Sequence[int]

[ktbarrett]

This works for my example, but of course doesn't allow you to say "It MUST be a subtype".

T_co = TypeVar("T")
U = TypeVar("U")

class ImmutableList(Generic[T]):
    def concat(self, item: U) -> ImmutableList[T | U]: ...
        
a: ImmutableList[int]
a.concat(1.0)

I figured this would work based on @ethanabrooks's use of a TypeVar in a class definition, but it doesn't?

class ImmutableList(Generic[T]):
    U = TypeVar("U", bound=T)
    def concat(self, item: U) -> ImmutableList[T | U]: ...

a: ImmutableList[int]
reveal_type(a.concat(1.0))
reveal_type(a.concat(1))

[Prometheus3375]

Some of the examples here have some issues.

def look_for_1_0(a: Sequence[float]) -> bool:
try:
a.index(1.0)
except IndexError:
return False
else:
return True

a: Sequence[int]
look_for_1_0(a) # passing a Sequence[int] should fail here as 1.0 can't be in a Sequence[int]

Sequence[int] is a subclass of Sequence[float] because int is considered a subclass of float and Sequence's generic is covariant. You can pass sequence of ints there absolutely, and my suggestion does not change anything in this example. Moreover, 1.0 == 1, so here it may not fail at all.

If my suggestion is considered, then type checker will highlight next cases:

seq: Sequence[int] = [1, 2, 3]
seq.index('str')
seq.index(2.2)
seq.index(object())

Currently none of them are highlighted. However, it will highlight seq.index(2.0) which returns true.

[ktbarrett]

Sequence[int] is a subclass of Sequence[float] because int is considered a subclass of float and Sequence's generic is covariant. You can pass sequence of ints there absolutely

That is exactly the problem.

def index(seq: Sequence[float], val: float) -> int:
    return seq.index(val)

seq: Sequence[int] = [1, 2, 3]
seq.index(2.2)  # Fails as expected
index(seq, 2.2)  # Passes unexpectedly

[Prometheus3375]

There is no problem as currently index accepts Any. And if "problem" is the fact that Sequence is covariant, there is none. In my example I am narrowing down type of value from Any to _T_co, but the fact that 1 == 1.0 == True makes it incorrect.

However, in a more customized examples where integral types are not allowed, specifying arguments of methods of immutable data structures as covariant still can be useful.