Add missing methods and @coroutine decorators to AbstractEventLoop #958

JelleZijlstra · 2017-02-27T03:45:03Z

Closes #953.

I reviewed the documentation at https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.AbstractEventLoop
and added missing methods and missing @coroutine decorators.

I ran mypy on the sample file from the issue report to confirm that mypy handles the combination
of @AbstractMethod and @coroutine correctly.

There are a number of types in this file that are annotated as Any but could be given more precise
types (e.g., sockets and protocol factories). I may submit another PR to fix those separately.

@AbstractMethod

Closes python#953. I reviewed the documentation at https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.AbstractEventLoop and added missing methods and missing @coroutine decorators. I ran mypy on the sample file from the issue report to confirm that mypy handles the combination of @AbstractMethod and @coroutine correctly. There are a number of types in this file that are annotated as Any but could be given more precise types (e.g., sockets and protocol factories). I may submit another PR to fix those separately.

gvanrossum · 2017-03-10T23:27:16Z

stdlib/3.4/asyncio/events.pyi

@@ -71,39 +80,52 @@ class AbstractEventLoop(metaclass=ABCMeta):
    @abstractmethod
    def call_soon_threadsafe(self, callback: Callable[..., Any], *args: Any) -> Handle: ...
    @abstractmethod
+    @coroutine
    def run_in_executor(self, executor: Any,
        callback: Callable[..., Any], *args: Any) -> Future[Any]: ...


If it's a coroutine it should not return a Future I think. All other coroutines are declared to return a Generator (the coroutine decorator then extracts something from that). Same for several more below.

The documentation at https://docs.python.org/3/library/asyncio-eventloop.html?highlight=run_in_executor#asyncio.AbstractEventLoop.run_in_executor says that this is a coroutine, but asyncio's concrete event loop implementation actually has them as functions that return Futures directly. It shouldn't matter to people calling those methods, so I'm going to keep the @coroutine decorator on them and change the return type to Generator to be consistent with the documentation for AbstractEventLoop.

gvanrossum · 2017-03-10T23:28:08Z

stdlib/3.4/asyncio/events.pyi

    def getnameinfo(self, sockaddr: tuple, flags: int = ...) -> Future[Tuple[str, int]]: ...
    @abstractmethod
+    @coroutine
    def create_connection(self, protocol_factory: Any, host: str = ..., port: int = ..., *,
                          ssl: Any = ..., family: int = ..., proto: int = ..., flags: int = ..., sock: Any = ...,
                          local_addr: str = ..., server_hostname: str = ...) -> tuple: ...


This tuple return should be something more specific, like Tuple[BaseTransport, BaseProtocol] perhaps?

I wanted to keep this out of this PR to avoid doing too much at once. There's a lot of Anys in this file that aren't necessary; I can fix those in this PR too if you prefer.

gvanrossum · 2017-03-10T23:29:28Z

stdlib/3.4/asyncio/events.pyi

    def create_connection(self, protocol_factory: Any, host: str = ..., port: int = ..., *,
                          ssl: Any = ..., family: int = ..., proto: int = ..., flags: int = ..., sock: Any = ...,
                          local_addr: str = ..., server_hostname: str = ...) -> tuple: ...
    @abstractmethod
+    @coroutine
    def create_server(self, protocol_factory: Any, host: str = ..., port: int = ..., *,
                      family: int = ..., flags: int = ...,
                      sock: Any = ..., backlog: int = ..., ssl: Any = ..., reuse_address: Any = ...) -> Any: ...


Returns a Server instance, actually.

gvanrossum · 2017-03-10T23:57:32Z

I don't mind fixing the return types (or at least many of them) in this PR.

I do hope you will look into the Future-vs-Generator issue I brought up, I'm not even sure why it works. (Maybe this is one of those cases where some runtime tests might have helped. :-)

JelleZijlstra · 2017-03-10T23:58:20Z

Yes, I'll definitely fix that when I have some more time to look at the code.

gvanrossum · 2017-03-13T03:20:33Z

Excellent!

gvanrossum · 2017-03-13T19:41:29Z

[Initially reported this incorrectly in https://github.com/python/mypy/pull/2926#issuecomment-286209314]

So I have some bad news.

Running mypy's own tests with these I see two new errors, one in crawl.py and on in crawl2.py. The offending line is

ipaddrs = yield from self.loop.getaddrinfo(host, port)

In the latter it uses await instead.

It's easy to repro manually:

$ mypy test-data/samples/crawl.py
test-data/samples/crawl.py:147: error: 'Union[Tuple[builtins.str, builtins.int], Tuple[builtins.str, builtins.int, builtins.int, builtins.int]]' object is not iterable

JelleZijlstra · 2017-03-14T02:42:22Z

It's because this PR changed the return type of loop.getaddrinfo: it now has Union[Tuple[builtins.str, builtins.int], Tuple[builtins.str, builtins.int, builtins.int, builtins.int]] and before it was just tuple. I copied the return type of socket.getaddrinfo (https://github.com/python/typeshed/blob/master/stdlib/3/socket.pyi#L334); the tuple is a 2-tuple for IPv4 or a 4-tuple for IPv6 (https://docs.python.org/3/library/socket.html#socket.getaddrinfo).

I think we exposed a mypy bug here, where it doesn't recognize that the value is iterable when both members of a union are iterable. This reproduces the bug:

from typing import Tuple, Union

def f(x: Union[Tuple[int], Tuple[int, str]]) -> None:
    y, *z = x  # E: 'Union[Tuple[builtins.int], Tuple[builtins.int, builtins.str]]' object is not iterable

I think we should report this as a mypy bug and ignore the error in test-data for now. Let me know if you have a different preference.

gvanrossum · 2017-03-14T15:19:33Z

Known bug, search the tracker. How shall we proceed?

JelleZijlstra · 2017-03-14T15:24:36Z

One reasonable option would be to change typeshed to not put the Union in getaddrinfo's return type. That might be best because I'm not even sure how to write any code using this Union of differently sized tuples that mypy will accept.

The other option would be to # type: ignore the test-data code. I can make a PR to do either one or the other (but probably not until tonight).

gvanrossum · 2017-03-14T15:37:55Z

We should fix the typeshed to not return the Union.

ambv · 2017-03-14T15:43:57Z

There's precedent of us removing Unions from return values to work around the fact that currently mypy doesn't build a "safe common denominator" of available members on types in a union. Typically we'd bail and return Any or a collection of any (which we could do here, Tuple[Any, ...]).

I don't oppose that we can do that here, too. However, once mypy gets smarter about this, we'll have to come back and somehow find the places where we did such workarounds. We haven't been marking them in the past. Maybe we could start now?

gvanrossum reviewed Mar 10, 2017

View reviewed changes

gvanrossum added the blocked label Mar 10, 2017

JelleZijlstra added 9 commits March 10, 2017 19:00

make sure all @coroutine functions return a Generator

d0f587e

get rid of some Tuple[Any] types, add alias for transport protocol pair

b5ef325

add type for protocol factories

8607c1b

add type for SSLContext

7733f6b

standardize arguments to Generator for @coroutine functions

8ee657b

concretize arguments to sock_* methods

2e361e3

add more socket arguments

aff835e

fix a few more Any return types

6e366ed

reuse_address is an Optional bool

69694d9

gvanrossum merged commit 3e94c46 into python:master Mar 13, 2017

JelleZijlstra mentioned this pull request Mar 13, 2017

Fix support for async for comprehensions python/mypy#2926

Merged

JelleZijlstra deleted the asynciocoro branch March 14, 2017 02:11

JelleZijlstra mentioned this pull request Mar 14, 2017

drop union return type from getaddrinfo #1002

Merged

gvanrossum removed the blocked label Mar 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add missing methods and @coroutine decorators to AbstractEventLoop #958

Add missing methods and @coroutine decorators to AbstractEventLoop #958

Uh oh!

JelleZijlstra commented Feb 27, 2017

Uh oh!

gvanrossum Mar 10, 2017

Uh oh!

JelleZijlstra Mar 11, 2017

Uh oh!

gvanrossum Mar 10, 2017

Uh oh!

JelleZijlstra Mar 10, 2017

Uh oh!

gvanrossum Mar 10, 2017

Uh oh!

gvanrossum commented Mar 10, 2017

Uh oh!

JelleZijlstra commented Mar 10, 2017

Uh oh!

gvanrossum commented Mar 13, 2017

Uh oh!

gvanrossum commented Mar 13, 2017

Uh oh!

JelleZijlstra commented Mar 14, 2017

Uh oh!

gvanrossum commented Mar 14, 2017

Uh oh!

JelleZijlstra commented Mar 14, 2017

Uh oh!

gvanrossum commented Mar 14, 2017

Uh oh!

ambv commented Mar 14, 2017

Uh oh!

Uh oh!

Uh oh!

Add missing methods and @coroutine decorators to AbstractEventLoop #958

Add missing methods and @coroutine decorators to AbstractEventLoop #958

Uh oh!

Conversation

JelleZijlstra commented Feb 27, 2017

Uh oh!

gvanrossum Mar 10, 2017

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra Mar 11, 2017

Choose a reason for hiding this comment

Uh oh!

gvanrossum Mar 10, 2017

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra Mar 10, 2017

Choose a reason for hiding this comment

Uh oh!

gvanrossum Mar 10, 2017

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Mar 10, 2017

Uh oh!

JelleZijlstra commented Mar 10, 2017

Uh oh!

gvanrossum commented Mar 13, 2017

Uh oh!

gvanrossum commented Mar 13, 2017

Uh oh!

JelleZijlstra commented Mar 14, 2017

Uh oh!

gvanrossum commented Mar 14, 2017

Uh oh!

JelleZijlstra commented Mar 14, 2017

Uh oh!

gvanrossum commented Mar 14, 2017

Uh oh!

ambv commented Mar 14, 2017

Uh oh!

Uh oh!