Skip to content

Document variance change in subclasses #3179

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

Closed
wants to merge 2 commits into from

Conversation

pkch
Copy link
Contributor

@pkch pkch commented Apr 15, 2017

The intent is to clarify that it's ok to change variance of type variables in subclasses; this is useful if someone wants to separate the interface into the parts that are covariant and contravariant in a type variable (rather than living with the overly-pessimistic invariance and/or overruling the type checker).

@pkch
Copy link
Contributor Author

pkch commented Apr 15, 2017

Summary of possible changes in variance (going down the inheritance hierarchy):

  • co/contra-variant to invariant is permitted; it's heavily used already, e.g., from Sequence (covariant) to MutableSequence (invariant)
  • contra- to co-variance or vice versa will only type check for trivial cases of an empty class - because if an attribute/method can be compatible with contravariance, it cannot also be compatible with covariance
  • invariance to co-/contra-variance will only type check if the invariance of the base class was unnecessary in the first place (e.g., if someone made a class invariant that actually only used the type variables in return types); in that case, presumably the author of the base class didn't think variance is safe for some non-reason unknown to mypy, so I think it's good to prohibit the change in variance

Edge case: Container and a few other collections.abc-inspried types. Container has no methods explicitly declared, so mypy doesn't complain. But through its metaclass (and semantics) we know that it has __contains__. This method should force Container to be contravariant, but it's declared as covariant. I think it should be fixed (it won't make a difference for mypy source code, but might help someone defining their own interface types).

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

I think it is to early to make this PR. We first need an issue where this could be discussed. This is not controversial from the theoretical point of view, but quite controversial from practical point of view (e.g. Container should be covariant).

Also, I would first updated PEP 483 - "Theory of type hints" (it actually contains the example of going from covariant to contravariant with empty bodies). Also PEP 483 allows a more detailed discussion (it is important to show an example when this is not safe).

Only after PEP 483 is updated and actual checks are implemented in mypy, then we could add this to mypy docs.


from typing import Generic, TypeVar, Tuple, Iterable

K = TypeVar('K')
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this definition of type variable and the one below should be exchanged.


from typing import Generic, TypeVar, Tuple, Iterable

K = TypeVar('K')
Copy link
Member

Choose a reason for hiding this comment

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

You want a covariant one here, probably.


K_co = TypeVar('K_co', covariant=True)
class InvariantSet(Generic[K], CovariantSet[K]):
def __contains__(self, key: K) -> bool: ...
Copy link
Member

Choose a reason for hiding this comment

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

This is different from how __contains__ is defined for e.g. Container, the latter has key: object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, interesting. At the same time, it uses type variable in Enum.Flag, stdlib/2/collections.pyi and lib-stub/typing.pyi

class InvariantSet(Generic[K], CovariantSet[K]):
def __contains__(self, key: K) -> bool: ...

but this is not:
Copy link
Member

Choose a reason for hiding this comment

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

But mypy does not prohibit this currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm it will prohibit it, if there are any methods or attributes in the base class at all; mypy will complain that they don't allow contravariance.

Copy link
Member

Choose a reason for hiding this comment

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

Currently it doesn't, and this is main reason why I think it is premature.
I just tried your new example (fixing key annotation in the subclass, it should be K_contra) and mypy doesn't complain.

@gvanrossum
Copy link
Member

Should this be closed until we've reached agreement on the feature???

@pkch
Copy link
Contributor Author

pkch commented Apr 22, 2017

Yes

@pkch pkch closed this Apr 22, 2017
@pkch
Copy link
Contributor Author

pkch commented Apr 23, 2017

Hmm @JukkaL you mentioned this exact thing here (item 3).

@ilevkivskyi
Copy link
Member

@pkch There is a separate issue #736

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.

3 participants