-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Use Literal overloads to give better types to subprocess #3110
Conversation
I tested this using:
|
This gives better types to subprocess.check_output and subprocess.run by laboriously overloading using literals. To support `run`, I turned `CompletedProcess` into `_CompletedProcess[T]` with `CompletedProcess = _CompletedProcess[Any]`. I could pretty easily be convinced that it would be better to just make `CompletedProcess` generic, though. I'd like to do the same for Popen but need to make mypy support believing the type of `__new__` in order for that to work.
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.
Thanks! Looks reasonable. I have few comments.
Generic, TypeVar, | ||
overload, | ||
) | ||
from typing_extensions import Literal |
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 think you can already import it from typing
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.
Apparently not yet?
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.
OK, I see, it is defined in typing
stub under if sys.version_info >= (3, 8): ...
I would prefer to also support respecting annotations on Overloaded constructors: T = TypeVar('bytes', 'Text')
class C(Generic[T]):
@overload
def __init__(self: C[Text], use_text: Literal[True]) -> None: ...
@overload
def __init__(self, use_text: bool) -> None: ... # allow omitting annotation for fallback case Methods that should be called only for some values of type argument: class C(Generic[T]):
def inc_value(self: C[int]) -> None: ...
x: C[int]
x.inc_value() # OK
y: C[str]
y.inc_value() # Error! Methods that have different signature depending on value of type argument: class C(Generic[T]):
@overload
def meth(self: C[int], inc_value: bool) -> None: ...
@overload
def meth(self) -> None: ... Complex relationship between types in method and type arguments: class C(Generic[T]):
@overload
def foo(self: C[int]) -> List[int]: ...
@overload
def foo(self: C[str]) -> str: ... The first one is of course the most important but we also have few issues about other things. |
This gives better types to subprocess.check_output and subprocess.run
by laboriously overloading using literals.
To support
run
, I turnedCompletedProcess
into_CompletedProcess[T]
with
CompletedProcess = _CompletedProcess[Any]
. I could pretty easilybe convinced that it would be better to just make
CompletedProcess
generic, though.
I'd like to do the same for Popen but need to make mypy support
believing the type of
__new__
in order for that to work.