-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Remove type cycle in click #2277
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
third_party/2and3/click/core.pyi
Outdated
@@ -313,6 +313,10 @@ class CommandCollection(MultiCommand): | |||
... | |||
|
|||
|
|||
# This type is here to resolve https://github.com/python/mypy/issues/5275 | |||
_ConvertibleType = Union[type, ParamType, Tuple[type, ...], Callable[[str], Any], Callable[[Optional[str]], Any]] |
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.
Could you put this alias in utils.pyi
so that you can avoid both an import cycle and the Any
you added in types.pyi
?
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.
I don't think that would fix the import cycle. It would just punt it down the road (utils would need to import types, types would need to import utils).
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.
Oh right, I didn't realize that this definition depends on ParamType.
In that case, we can get around this by moving ParamType into utils.pyi, prefixed with an underscore to make it private. In types.pyi, we can then from .utils import _ParamType as ParamType
.
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.
That seems not to fix it either. Based on the minmal repro I just found, I think Parameter and ParamType need to be in the same file. I am about to push something that passes (if you would like me to move things around, while keeping these two classes in the same file, let me know).
This works around python/mypy#5275
It sadly introduces an
Any
, but I felt that it is an acceptable compromise. Let me know if you have a different opinion on where theAny
should be introduced.