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

Variance of TypeVar is incompatible with base class #4111

Closed
LeeeeT opened this issue Oct 30, 2022 · 11 comments
Closed

Variance of TypeVar is incompatible with base class #4111

LeeeeT opened this issue Oct 30, 2022 · 11 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@LeeeeT
Copy link

LeeeeT commented Oct 30, 2022

Pyright gives me an error for the following code:

from typing import Generic, TypeVar


T_contra = TypeVar("T_contra", contravariant=True)


class Base(Generic[T_contra]):
    def method(self, arg: T_contra) -> None:
        ...


class Derived(Base[frozenset[T_contra]], Generic[T_contra]):  # error here
    def method(self, arg: frozenset[T_contra]) -> None:
        ...
Type "T_contra@Derived" cannot be assigned to type variable "_T_co@frozenset"
    Variance of TypeVar "T_contra@Derived" is incompatible with base class "frozenset"

However, I think this code is correct. The current annotations mean I can assign:

  • Derived[float] to Derived[int];
  • (arg: frozenset[float]) -> None to (arg: frozenset[int]) -> None;
  • frozenset[int] to frozenset[float];
  • int to float.

Surprisingly, if I use covariant TypeVar in Derived...

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


class Derived(Base[frozenset[T_co]], Generic[T_co]):
    def method(self, arg: frozenset[T_co]) -> None:
        ...

...I get no errors at all. But this code is completely wrong since it violates Liskov substitution principle.

pyright 1.1.277

@erictraut
Copy link
Collaborator

Pyright is correct here, so this isn't a bug. The variance of T_contra is inconsistent with the variance of the TypeVar for frozenset, so you cannot use it in this location.

You need to use TypeVars with different variances for Base and Derived, like this:

from typing import Generic, TypeVar

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


class Base(Generic[T_contra]):
    def method(self, arg: T_contra) -> None:
        ...

class Derived(Base[frozenset[T_co]]):
    def method(self, arg: frozenset[T_co]) -> None:
        ...

@erictraut erictraut added the as designed Not a bug, working as intended label Oct 30, 2022
@LeeeeT
Copy link
Author

LeeeeT commented Oct 31, 2022

Your code violates Liskov substitution principle. Here's why:

from typing import Generic, TypeVar

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


class Base(Generic[T_contra]):
    def method(self, arg: T_contra) -> None:
        ...

class Derived(Base[frozenset[T_co]]):
    def method(self, arg: frozenset[T_co]) -> None:
        ...

class DerivedInt(Derived[int]):
    def method(self, arg: frozenset[int]) -> None:
        print("expected subtype of int, got:", next(iter(arg)).__class__.__name__)


def f(d: Derived[float]) -> None:
    d.method(frozenset({3.14}))


f(DerivedInt())  # there should be an error
# but there isn't, because Derived is covariant

Prints:

expected subtype of int, got: float

@erictraut erictraut added needs investigation Requires additional investigation to determine course of action and removed as designed Not a bug, working as intended labels Oct 31, 2022
@erictraut erictraut reopened this Oct 31, 2022
@erictraut
Copy link
Collaborator

erictraut commented Nov 6, 2022

Thinking about this more, you are correct that pyright should not allow frozenset[T_co] as a type argument for Base but should accept frozenset[T_contra].

This will be fixed in the next release.

@erictraut erictraut added bug Something isn't working and removed needs investigation Requires additional investigation to determine course of action labels Nov 6, 2022
erictraut pushed a commit that referenced this issue Nov 6, 2022
@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Nov 6, 2022
@erictraut
Copy link
Collaborator

This is included in pyright 1.1.279, which I just published. It will also be included in a future version of pylance.

@LeeeeT
Copy link
Author

LeeeeT commented Mar 15, 2023

@erictraut, the same bug is happening again after upgrading to pyright 1.1.298.

@erictraut erictraut reopened this Mar 15, 2023
@erictraut erictraut removed the addressed in next version Issue is fixed and will appear in next published version label Mar 15, 2023
@erictraut
Copy link
Collaborator

@LeeeeT, hmm, that's strange. I added a unit test to detect regressions when I fixed this previously. I take a closer look. Reopening the issue.

@LeeeeT
Copy link
Author

LeeeeT commented Mar 16, 2023

@erictraut, I created the following exhaustive test case:

from typing import Generic, TypeVar

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

class Co(Generic[T_co]): ...

class Contra(Generic[T_contra]): ...


class CoToContra(Contra[Co[T_contra]]): ...

class ContraToContra(Contra[Contra[T_co]]): ...

class CoToCo(Co[Co[T_co]]): ...

class ContraToCo(Co[Contra[T_contra]]): ...

It ensures the following rules (preudocode):

  • Co[Contra] -> Contra
  • Contra[Co] -> Contra
  • Co[Co] -> Co
  • Contra[Contra] -> Co

So, you can think about it like this:

  • When you parameterize a covariant or a contravariant type with a type of the same variance you get a covariant type back.
  • Similarly, when you parameterize a type with a type of the opposite variance you get a contravariant type back.

For the test case above pyright gives errors for all 4 classes.

Interestingly, if I get rid of inheritance in this example the errors disappear:

from typing import Generic, TypeVar

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

class Co(Generic[T_co]): ...

class Contra(Generic[T_contra]): ...


class CoToContra(Generic[T_contra]):
    def f(self, arg: Co[T_contra]) -> None: ...

class ContraToContra(Generic[T_co]):
    def f(self, arg: Contra[T_co]) -> None: ...

class CoToCo(Generic[T_co]):
    def f(self) -> Co[T_co]: ...

class ContraToCo(Generic[T_contra]):
    def f(self) -> Contra[T_contra]: ...

This code ensures the same rules but without using inheritance (uses functions instead to trigger variance checking). Maybe something is going wrong in pyright when handling inheritance?

@erictraut
Copy link
Collaborator

This will be fixed in the next release. I incorporated your test case — thanks!

erictraut pushed a commit that referenced this issue Mar 16, 2023
@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Mar 16, 2023
@erictraut
Copy link
Collaborator

This is included in pyright 1.1.300, which I just published. It will also be included in a future release of pylance.

@LeeeeT
Copy link
Author

LeeeeT commented Mar 22, 2023

Sorry, @erictraut, your fix didn't help. You incorporated my second test example which, as I said, worked fine. The one that uses inheritance is the one that was failing in pyright 1.1.298 and is still failing on version 1.1.300, but now pyright gives errors for only 2 classes, not 4.

@erictraut
Copy link
Collaborator

Please open a new bug with a clear example of what you think is going wrong. This one has already been marked fixed and closed twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants