Overload the constructor of subprocess.Popen #3113
Conversation
This takes advantage of a recent mypy change to respect the return type of `__new__`. Using that it does the same tedious overloads as `run` and `check_output`.
stdlib/3/subprocess.pyi
Outdated
| @@ -808,96 +808,347 @@ class CalledProcessError(Exception): | |||
| output: Optional[_TXT] = ..., | |||
| stderr: Optional[_TXT] = ...) -> None: ... | |||
|
|
|||
| class Popen: | |||
| class _Popen(Generic[AnyStr]): | |||
There was a problem hiding this comment.
Actually now I am not sure underscore+alias is actually needed (and was probably not needed for CompletedProcess) since these types will appear in error messages and may confuse people. But let's see what other people think @JelleZijlstra @srittau
There was a problem hiding this comment.
I'm going to drop it
| pid = 0 | ||
| returncode = 0 | ||
|
|
||
| # Technically it is wrong that Popen provides __new__ instead of __init__ | ||
| # but this shouldn't come up hopefully? |
There was a problem hiding this comment.
Sure, the only way to know how important it is to be precise is to try it, so I propose to ahead.
There was a problem hiding this comment.
Yuck, so if anyone overrides __init__ (like some of our code does to provide additional features), then the returned type is wrong.
What's the reason __new__ needs to be used over __init__?
There was a problem hiding this comment.
I'm guessing the reason for using __new__ is that with __init__, there wouldn't be a way to specify Popen's contained type because it doesn't come directly from any of the parameter types.
For what it's worth, the way pytype gets around this kind of problem is with a pyi extension we refer to as "mutations", e.g.:
class Popen:
@overload
def __init__(self, ...):
self = Popen[str]
@overload
def __init__(self, ...):
self = Popen[bytes]
On a related note, I just finished rolling out this change at Google, and while the __new__/__init__ distinction caused a few false positives, the more precise Popen types flagged tons of places where subprocess output was being used in Python 3-incompatible ways, which was incredibly useful.
This takes advantage of a recent mypy change to respect the return type of `__new__`. Using that it does the same tedious overloads as `run` and `check_output`.
This takes advantage of a recent mypy change to respect the return
type of
__new__. Using that it does the same tedious overloadsas
runandcheck_output.