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

Reject bound covariant type variable in an argument type #734

Open
JukkaL opened this issue Aug 9, 2015 · 12 comments
Open

Reject bound covariant type variable in an argument type #734

JukkaL opened this issue Aug 9, 2015 · 12 comments

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Aug 9, 2015

We should reject a bound covariant type variable in an argument type (i.e., in a contravariant position). For example, this should be rejected:

T = TypeVar('T', covariant=True)
class A(Generic[T]):
    def foo(self, x: T) -> None: ...  # Can't use as argument type! Return type is okay.

Similarly, a contravariant type variable should not be used in a return type.

The actual rule is probably a bit more involved than what I gave above, but this would probably cover most cases.

@JukkaL JukkaL added the feature label Aug 9, 2015
jhance added a commit to jhance/mypy that referenced this issue Oct 10, 2015
Reject covariant type variables being used as an argument
type. Reject contravariant type variables being used as
a return type.

Fixes python#734.
jhance added a commit to jhance/mypy that referenced this issue Oct 10, 2015
Reject covariant type variables being used as an argument
type. Reject contravariant type variables being used as
a return type.

Fixes python#734.
@jhance
Copy link
Collaborator

jhance commented Oct 11, 2015

Likely we don't want to be able to take Sequence[T] as an argument either. My initial PR doesn't address this, and I'm currently working on other covariance/contravariance which needs the same type of recursion, so I think it makes sense to address that (more complicated) issue later.

I'm actually not sure whether we would reject List[T] as an argument or not.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Oct 12, 2015

I think that List[T] as an argument should be rejected. Consider this example:

CV = TypeVar('CV', covariant=True)
IV = TypeVar('IV')

class A(Generic[CV]):
    def get(self) -> CV: ...
    def f(self, a: List[CV]) -> None: ...

class B(A[IV], Generic[IV]):
    def __init__(self, x: IV) -> None:
        self.x = x

    def get(self) -> IV:
        return self.x

    def f(self, a: List[IV]) -> None:
        self.x = a[0]

b = B(1)
a = b # type: A[object]
a.f([object()])
b.get() # ouch, object(), but should be int

@JukkaL JukkaL closed this as completed in 5fc466b Oct 12, 2015
JukkaL added a commit that referenced this issue Oct 12, 2015
Fixes for covariance/contravariance + #734
@JukkaL
Copy link
Collaborator Author

JukkaL commented Oct 12, 2015

Reopening because the List[T] case is still open.

@JukkaL JukkaL reopened this Oct 12, 2015
@JukkaL JukkaL added topic-type-variables priority-1-normal bug mypy got something wrong and removed feature labels May 18, 2018
@Emerentius
Copy link

I don't understand why this should be rejected or what to do in its place.

T = TypeVar('T', covariant=True)
class A(Generic[T]):
    def foo(self, x: T) -> None: ...  # Can't use as argument type! Return type is okay.

There is nothing wrong with wanting to take a T argument in a method of a covariant generic class[T], it's only that the function type of foo shouldn't be covariant over its input variable.

As far as I can see, you'd either need a way to define new type variables that are bounded on another type variable or maybe the variance part of the type variable should only apply to the first type constructor it is used in.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Oct 9, 2019

@Emerentius It can result in type unsafety if there is a subclass of A[T] that is invariant.

@Emerentius
Copy link

I can see it, but it's a combination of Covariance && Inheritance && Changing Variance in the child. I suppose we'll have to wait until PEP 591 to allow this?

@JukkaL
Copy link
Collaborator Author

JukkaL commented Oct 10, 2019

Mypy already supports final classes and methods, so there's no need to wait for PEP 591.

I think that it may be fine to allow that for final classes and methods, but I'd like to see some real use case first that would benefit from it. @ilevkivskyi What do you think about this?

@ilevkivskyi
Copy link
Member

I think there is a confusion between two things: and obvious bug, and a missing low-priority feature. The bug is that this code is clearly safe (because the variable S is not bound by class C):

from typing import Generic, TypeVar
  
S = TypeVar('S', covariant=True)
T = TypeVar('T', covariant=True)

class C(Generic[T]):
    def meth(self, x: S) -> S:  # mypy complains here about variance
       ...

The workaround is trivial, just define another type variable (without covariant=True) if S is used elsewhere. About the missing feature, I agree with

I'd like to see some real use case first that would benefit from it

@Emerentius
Copy link

My usecase is recreating something akin to Rust's Result type.

For this minimal example I'm using the Option type which has the same problem with fewer distracting type variables:

from typing import TypeVar, Union, Generic

T = TypeVar('T', covariant=True)

class Some(Generic[T]):
    def __init__(self, value: T) -> None:
        self._value = value

    def unwrap_or(self, _value: T) -> T:
        return self._value

class Nothing:
    def unwrap_or(self, value: T) -> T:
        return value

Option = Union[Some[T], Nothing]

a: Option[int] = Nothing()
a.unwrap_or(3) # ok
a.unwrap_or("3") # not ok

here, Some is supposed to be an immutable type and it should be possible to make it covariant.

A bit closer to typical python, a hypothetical FrozenDict[K, V] type could be covariant over V and have a get method: def get(self, key: K, default: V) -> V. This is very similar to the unwrap_or in that it has an argument that flows directly into the output and isn't stored. In both cases it's a fallback for something null-like, but that's coincidental when it comes to the variance question.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Oct 11, 2019

Based on a cursory inspection, the Optional example seems reasonable, as long as Some is final and the _value attribute is final. The details of how to support the example without compromising type safety in other use cases would still require some careful thought.

@sobolevn
Copy link
Member

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Sep 26, 2023

@JukkaL Given that the example in the OP is now correctly handled by mypy, could it be updated to a case that is not? e.g.

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

class A(Generic[out_T]):
    def f(self, l: list[out_T]) -> None:
        self.value = l[0]

a_int = A[int]()
a_object: A[object] = a_int

a_object.f(["not an int"])
a_int.value + 1

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.

6 participants