Skip to content

Support for covariant and contravariant TypeVar's #694

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

Merged
merged 19 commits into from
Aug 9, 2015

Conversation

spkersten
Copy link
Contributor

This PR adds support for covariant and contravariant type variables, like:

from typing import TypeVar, Generic

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

class G(Generic[T]): pass
class A: pass
class B(A): pass
class C(B): pass

a = None  # type: G[A]
b = None  # type: G[B]
c = None  # type: G[C]

b = a  # This is OK: G[A] is a subtype of G[B] 
       # because B is a subtype of A and 
       # G in contravariant in its type parameter
b = c  # E: Incompatible types in assignment 
       # (expression has type G[C], variable has type G[B])

See the test cases I added for other examples.

Note that also the default has changed from covariant to invariant (as specified by the PEP). I made the minimal changes to typing.py to make it work (the type variables were covariant by default and I've defined them now explicitly as such). But we should probably copy in the version from ambv/typehinting.

There are still a few things to do (see below), but I won't be able to work on this until Tuesday and I don't expect big changes so it can be reviewed.

Todo

  • More tests
    • Especially for code paths that use is_proper_subtype, for which there are no tests at all currently.
    • Also subtype test; now there're only tests for covariant type variables.
  • Fix type error (Cannot determine type of 'INVARIANT')
  • In the test cases, replace True = 1 with something better
  • In semanal.py, there are several functions that return a List[Tuple[str, TypeVarExpr]], where the string is the type variable name. The string can probably be refactored out, as TypeVarExpr already contains the name. -> Names are not identical (qualified vs. unqualified it seems)

Fixes #643.

_T = TypeVar('_T')
_KT = TypeVar('_KT')
_VT = TypeVar('_VT')
_T = TypeVar('_T', covariant=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutable collections should have invariant type variables (e.g., MutableSequence).

@JukkaL
Copy link
Collaborator

JukkaL commented May 26, 2015

Thanks for implementing this! This is important as this is one of the biggest features missing from PEP 484 support.

I did a quick pass and it looks pretty good!

There are a bunch of additional things that should probably go in at some point. It's probably best to leave these out of this PR, as the current PR is already a significant improvement over what was before.

(1)

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.

(2)

Like (1), but for attributes. In a covariant class an instance attribute with type List[T] would not be valid, since List is invariant.

(3)

In generic inheritance, we can't change the variance of a type from invariant to covariant/contravariant. This should probably be rejected:

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

class A(Generic[IV]): ... # Bunch of methods
class B(A[CV], Generic[CV]): pass  # Error, type variable should be invariant

Going from covariant to invariant is fine (for example, List has to do this since it subclasses Sequence, which is covariant).

(4)

Covariant type variable should not be used in the return type in an invariant position, for example like this:

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

class A(Generic[CV]):
    def foo(self) -> List[CV]: ...   # Error, should be Sequence[CV] or similar, since List is invariant

There is probably a similar rule for contravariance.

Notes:

There are probably some additional corner cases, but the above should cover the most interesting bits. And I'm not 100% sure that the above are correct... haven't thought about this very carefully yet. Don't hesitate to ask if some the above doesn't make any sense -- this is actually a pretty tricky feature :-)

@spkersten
Copy link
Contributor Author

@JukkaL I agree that it's best to leave those additional checks out and have this PR focussed on the relations between generic types.

I've started updating testtypes.py; that will take more time than I expected. That also led me to realise that I have to update meet_types() and join_types() as well. I want to add that to this PR as it seems like a small change and would leave them broken otherwise.

@spkersten
Copy link
Contributor Author

I made a few fixes to meet_types and join_types, but to make them fully aware of the variance of type parameters is a bit more tricky (they'll depend on each other). So I propose to keept it like this for this PR. The only thing I wasn't able to resolve it the need for True = 1 in the test cases. I don't think it's a big problem, but if you know how to fix it I'll do it. Other than that, this can be merged as far as I'm concerned.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 15, 2015

Cool, thanks for the updates.

Things are currently seriously hectic in my life, so I can't promise that I'll be able to review this very soon. I'm moving away from my current place in two weeks and after that I'll be traveling for another two weeks (and I'll need to prepare two workshop talks about mypy that I'll give at ECOOP in Prague), but I'll try to find some time.

@spkersten
Copy link
Contributor Author

Take your time and good luck with your workshops.

@o11c
Copy link
Contributor

o11c commented Jun 30, 2015

Are you planning on working with #689 as well?

@JukkaL JukkaL merged commit 18a2b15 into python:master Aug 9, 2015
@JukkaL
Copy link
Collaborator

JukkaL commented Aug 9, 2015

Finally had the time to finish reviewing this! I did some updates to stubs to avoid having covariant type variables in argument types. I'll add further work (as discussed above) as separate issues.

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

Successfully merging this pull request may close these issues.

Support covariant and contravariant TypeVar keyword arguments
3 participants