python / typing-council

Decisions by the Python Typing Council
43 stars 3 forks source link

Do not reject use of covariant or contravariant TypeVars in a function #8

Closed erictraut closed 10 months ago

erictraut commented 10 months ago

The typing spec contains some outdated language indicating that type checkers should reject the use of a TypeVar that is explicitly defined as "covariant" or "contravariant" if it is bound to a generic function. (It doesn't say anything about a generic type alias, but presumably it intended for the same restriction in this case.)

Variance has no meaning for a TypeVar that is bound to a generic function or type alias, so it makes more sense for type checkers to simply ignore the variance in this case (regardless of whether the TypeVar is invariant, covariant or contravariant). This is what all major type checkers do already.

This change therefore brings the spec in alignment with the current implementations.

Here's the PR for this change, which updates the spec and conformance tests: https://github.com/python/typing/pull/1576. (UPDATE: This was accidentally merged. That was reverted and a new draft exists: https://github.com/python/typing/pull/1587.)

Here is the discussion about the change.

gvanrossum commented 10 months ago

This is what all major type checkers do already.

Not mypy apparently: playground link.

While I wouldn't find this a great loss, I don't think it does our users a favor: there is plenty of confusion about variance, and a user might be forgiven for thinking that using e.g. a covariant typevar in a generic function has some specific meaning. Since there's a convention of naming typevars to convey their variance (e.g. T_co), silently accepting this would just allow confused users to spread the confusion.

erictraut commented 10 months ago

The error you're seeing in the playground example is a different check (and a different error message) than the one I'm talking about here. Mypy always emits an error when a "naked" covariant TypeVar is used to annotate an input parameter. This is independent of whether it's a function-scoped or class-scoped TypeVar.

Here's a modified example that doesn't use a "naked" covariant TypeVar but instead uses the type list[Tco]. By the current spec, this should result in an error indicating that a "covariant or a contravariant TypeVar cannot be bound to a generic function" (or something to that effect). Mypy has never generated such an error, as far as I can tell. Adding this now would undoubtedly produce a lot of noise with little or no benefit to users.

hauntsaninja commented 10 months ago

Thanks for clarifying, the difference there is a little subtle...

I'm on board with the broader idea here. Note the specific wording in https://github.com/python/typing/pull/1576 still seems to disallow the diagnostic Guido mentions, since mypy is very much not ignoring the declared variance. Since this is something a type checker already implements and is probably somewhat clarifying to users (even if a little ad hoc or potentially unintentional), my instinct is for that diagnostic to not be disallowed.

The most important thing to standardise is how annotated libraries are interpreted. Maybe the wording could be:

Covariance or contravariance is not a property of a type variable. Variance is meaningful only when a type variable is bound to a generic class. If a type variable with declared variance is bound to a generic function or type alias, type checkers may warn users about this. However, any subsequent type analysis involving such functions or aliases should proceed ignoring the declared variance.

That said, thanks to PEP 695 providing a more general solution, I don't have a strong opinion here.

erictraut commented 10 months ago

I'm fine with that wording.

erictraut commented 10 months ago

Actually, the sentence "Covariance or contravariance is not a property of a type variable" is problematic and misleading. The variance is a property of a type variable, but it's meaningful only when the type variable is bound to a generic class. I propose that we simply delete that sentence in the above paragraph.

Also, the phrase "declared variance" may be misleading and ambiguous. Technically, all TypeVars (prior to PEP 695) have a declared variance. They're declared as either covariant, contravariant, or invariant.

rchen152 commented 10 months ago

While I wouldn't find this a great loss, I don't think it does our users a favor: there is plenty of confusion about variance, and a user might be forgiven for thinking that using e.g. a covariant typevar in a generic function has some specific meaning. Since there's a convention of naming typevars to convey their variance (e.g. T_co), silently accepting this would just allow confused users to spread the confusion.

+1 to this, TypeVars are confusing enough that I don't like the idea of specifying that type checkers should silently accept something that can spread misunderstandings.

Saying that the variance is meaningless in this case seems perfectly reasonable and a good point to clarify, as long as we don't close the door on a type checker implementing additional checks.

erictraut commented 10 months ago

@JelleZijlstra, any remaining concerns or points that you'd like to discuss? If you agree with the current wording, please sign off on the proposal.

JelleZijlstra commented 10 months ago

Looks good! I'll spend some time today catching up on various spec changes.

JelleZijlstra commented 10 months ago

We all signed off, so this is approved.