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

Updated spec and conformance test to reflect that a type variable tha… #1587

Merged
merged 3 commits into from
Jan 21, 2024

Conversation

erictraut
Copy link
Collaborator

…t is defined as covariant or contravariant should not generate an error when bound to a generic function or type alias. Variance has no meaning in this context, so it should be ignored. This brings the spec in alignment with the behavior of the major type checkers.

…t is defined as covariant or contravariant should not generate an error when bound to a generic function or type alias. Variance has no meaning in this context, so it should be ignored. This brings the spec in alignment with the behavior of the major type checkers.
@hauntsaninja
Copy link
Collaborator

Linking python/typing-council#8

For example, the following example is
fine::
Variance is meaningful only when a type variable is bound to a generic class.
Variance has no meaning, and should therefore be ignored by type checkers, if
Copy link
Member

Choose a reason for hiding this comment

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

I'd be okay with this PR if you just remove "should therefore be ignored w by type checkers", so type checkers may choose to reject typevars declared as covariant or contravariant in contexts where variance has no meaning.

Suggested change
Variance has no meaning, and should therefore be ignored by type checkers, if
Variance has no meaning if

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If type checkers do not ignore variance when it has no meaning, what should they do instead? Keep in mind that every TypeVar (prior to PEP 695's "inferred variance") has an explicit variance; it's either invariant, covariant, or contravariant.

Copy link
Member

Choose a reason for hiding this comment

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

Since "invariant" is declared by not adding {co,contra}variant=True to the TypeVar() call, I'd propose that type checkers accept invariant (i.e., default) typevars but issue an error for co/contra-variant typevars.

I believe it is reasonable to assume that omitting any mention of variance in the TypeVar() call is less explicit than explicitly using one of the two variance-declaring options (there is no invariant=True option).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was curious, so I quickly implemented a check in pyright for a TypeVar that's covariant or contravariant if it is scoped to a generic function or type alias. Here's the mypy_primer result: microsoft/pyright#6993. This isn't as noisy as I predicted. Interestingly, 100% of the violations it caught were in type aliases rather than functions.

I think we should make it clear in the spec that variance has no meaning for TypeVars scoped to functions or type aliases — and that type checkers should therefore ignore the variance for purposes of type checking. As for additional checks, I think we have three options:

  1. Indicate that type checkers should warn users if they attempt to use a covariant or contravariant TypeVar scoped to a generic function or type alias.
  2. Indicate that type checkers may warn users in this circumstance (but not mandate it).
  3. Indicate that type checkers should be silent about this circumstance, since variance is meaningless in this context.

I guess I don't have a strong opinion. The current draft text aligns to 3. It sounds like you prefer 1? Or 2?

I do think that mypy's current behavior is hard to defend. It emits an unrelated (and kind of meaningless) error if a covariant TypeVar is used for an input parameter or if a contravariant TypeVar is used for a return type. Since variance isn't meaningful in this case, those errors are inappropriate and confusing, IMO.

Copy link
Member

Choose a reason for hiding this comment

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

I'd go for (2), or (2a) don't allude to what type checkers should do at all.

(I'm beginning to realize that "has no meaning" is a rather ambiguous phrase, which implies different things in different contexts -- at least to me. I don't think it's the same as "is redundant" here.)

I agree that mypy's error is not what I thought it was and should be fixed independently.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

LG on the spec text now.

@erictraut
Copy link
Collaborator Author

@gvanrossum, if you're OK with this wording, please sign off on the TC decision here: python/typing-council#8. Thanks!

@erictraut
Copy link
Collaborator Author

@JelleZijlstra & @rchen152, let me know if you have any suggested improvements. If you're OK with the latest wording, please sign off on the TC decision.

…variance_2

# Conflicts:
#	conformance/results/pyright/generics_variance.toml
#	conformance/results/results.html
@erictraut erictraut marked this pull request as ready for review January 21, 2024 23:26
@erictraut erictraut merged commit 53846cc into python:main Jan 21, 2024
4 checks passed
@erictraut erictraut deleted the typevar_variance_2 branch January 21, 2024 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants