-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Replacing Callable[[T], T] with callback protocol in click #4045
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds sensible.
I am not sure I like this change and I am unsure why pyre would like to introduce this arbitrary restriction. I find (Also, per our conventions, this would need to be called |
As this is quite a significant change, I think it's best discussed at typing-sig. |
This isn't quite introducing a new restriction as much as codifying our pre-existing lack of support for a MyPy-specific special case. Pyre has always interpreted all free type variables in a
Pyre and MyPy only differ here in that MyPy introduces a unique special case that alters the behavior for free variables:
I can definitely see how this special case is helpful in making definitions for decorator factories more compact, but for us the inconsistency of this behavior from a user's perspective outweighs the concision. Happy to forward this discussion onto typing-sig if you think it would be helpful. |
@srittau checking back in: do you still think that this needs to be discussed on typing-sig, or are you satisfied with the explanation above? |
@mrkmndz Sorry for forgetting to answer. Yes, considering theconsequences this has for the typing infrastructure, I think this should be discussed on typing-sig. |
OK, moving this discussion to |
Hi @srittau , would you mind responding to @dkgi 's question of you on typing-sig? I think we're both still a bit confused as to what precisely is blocking this PR. Thanks! |
I am honestly not sure what else there is to add. So far, |
I just replied on the email thread; I think we should go ahead with this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do this for now, although, as mentioned here and on typing-sig, I'd like _IdentityFunction
to be moved to _typeshed.pyi
.
Summary: This behavior is confusing and slightly non-standard (python/typeshed#4045), thus it ought to be explained in detail on this page. Pull Request resolved: #297 Reviewed By: shannonzhu Differential Revision: D22964776 Pulled By: mrkmndz fbshipit-source-id: ae3b168049d84cb721762ce6ac6d2c8a0c05e187
We are trying to strictify the type variable scoping semantics in Pyre such that
class
es and the parameters ofdef
s are the only places in which new type variable scopes can be created.This would mean that things like
would be rejected with
Invalid type variable [34]: The type variable `Variable[T]` isn't present in the function's parameters.
We currently don't error here, but don't handle this consistently correctly.
MyPy currently supports this (mod python/mypy#3924), and we don't necessarily need that behavior to change. Instead we'd just like typeshed to reflect the lowest common denominator between our two semantics: callback protocols.
For the above example, both MyPy and Pyre support this alternative spelling:
This PR converts a series of decorators in click that look like the first example into spellings like the second example. If this is agreeable to the community, we'd like to replicate this throughout typeshed.