Popen.communicate() morally wrong

[walles]

That's what the comments say at least, and I think I agree.

So I tried to fix it, but got this running python3 tests/mypy_selftest.py:

[...]
passed 129, failed 1, pending 0; running 0

FAILURE  #6 check package mypy

mypy/test/testpythoneval.py:134: error: Argument 1 to "split_lines" has incompatible type "Union[bytes, str, None]"; expected "bytes"
mypy/test/testpythoneval.py:134: error: Argument 2 to "split_lines" has incompatible type "Union[bytes, str, None]"; expected "bytes"

SUMMARY  1/130 tasks and 1/276 tests failed

Here's the patch I tried to apply:

diff --git a/stdlib/2/subprocess.pyi b/stdlib/2/subprocess.pyi
index 047b6bd..1be793e 100644
--- a/stdlib/2/subprocess.pyi
+++ b/stdlib/2/subprocess.pyi
@@ -95,8 +95,7 @@ class Popen:
 
     def poll(self) -> int: ...
     def wait(self) -> int: ...
-    # morally: -> Tuple[Optional[bytes], Optional[bytes]]
-    def communicate(self, input: Optional[_TXT] = ...) -> Tuple[Any, Any]: ...
+    def communicate(self, input: Optional[_TXT] = ...) -> Tuple[Optional[bytes], Optional[bytes]]: ...
     def send_signal(self, signal: int) -> None: ...
     def terminate(self) -> None: ...
     def kill(self) -> None: ...
diff --git a/stdlib/3/subprocess.pyi b/stdlib/3/subprocess.pyi
index c0a8613..0787380 100644
--- a/stdlib/3/subprocess.pyi
+++ b/stdlib/3/subprocess.pyi
@@ -325,14 +325,12 @@ class Popen:
     if sys.version_info >= (3, 3):
         def communicate(self,
                         input: Optional[_TXT] = ...,
-                        timeout: Optional[float] = ...,
-                        # morally: -> Tuple[Optional[_TXT], Optional[_TXT]]
-                        ) -> Tuple[Any, Any]: ...
+                        timeout: Optional[float] = ...
+                        ) -> Tuple[Optional[_TXT], Optional[_TXT]]: ...
     else:
         def communicate(self,
-                        input: Optional[_TXT] = ...,
-                        # morally: -> Tuple[Optional[_TXT], Optional[_TXT]]
-                        ) -> Tuple[Any, Any]: ...
+                        input: Optional[_TXT] = ...
+                        ) -> Tuple[Optional[_TXT], Optional[_TXT]]: ...
     def send_signal(self, signal: int) -> None: ...
     def terminate(self) -> None: ...
     def kill(self) -> None: ...

And here's the source code that the complaint was about:

def split_lines(*streams: bytes) -> List[str]:
    """Returns a single list of string lines from the byte streams in args."""
    return [
        s.rstrip('\n\r')
        for stream in streams
        for s in str(stream, 'utf8').splitlines()
    ]

...

def run(
    cmdline: List[str], *, env: Optional[Dict[str, str]] = None, timeout: int = 300
) -> Tuple[int, List[str]]:
    """A poor man's subprocess.run() for 3.3 and 3.4 compatibility."""
    process = subprocess.Popen(
        cmdline,
        env=env,
        stdout=subprocess.PIPE,
        stderr=subprocess.PIPE,
        cwd=test_temp_dir,
    )
    try:
        out, err = process.communicate(timeout=timeout)
    except subprocess.TimeoutExpired:
        out = err = b''
        process.kill()
    return process.returncode, split_lines(out, err)

The error is about the last line passing Union[bytes, str, None] into split_lines(), which expects a bytes.

Since I don't know what to do here (and I would like to get rid of the Anys), I'm filing this ticket.

Help!

Or at least some comments about what the "morally wrong" Anys are doing there would be nice :).

/Johan

[JelleZijlstra]

This is related to the general rule against Union return types (Optional is a kind of Union): python/mypy#1693.

[srittau]

I can't see what typeshed can do differently here. Returning Optional[bytes] would be more correct, but has the practical problem that the caller would need to assert that the return values are not None.