asyncio.events run_in_executor has incorrect return type annotation

[jamesba]

The following line in asyncio/events.pyi:

@abstractmethod
    async def run_in_executor(self, executor: Any,
                              func: Callable[..., _T], *args: Any) -> _T: ...

Does not match the actual observed behaviour of this method at runtime:

Python 3.6.9 (default, Apr 18 2020, 01:56:04)
[GCC 8.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> loop = asyncio.get_event_loop()
>>> def do_nothing():
...     pass
...
>>> f = loop.run_in_executor(None, do_nothing)
>>> f
<Future pending cb=[_chain_future.<locals>._call_check_cancel() at /usr/lib/python3.6/asyncio/futures.py:403]>
>>> type(f)
<class '_asyncio.Future'>
>>>

Suggest that the type annotation should instead be:

@abstractmethod
    def run_in_executor(self, executor: Any,
                              func: Callable[..., _T], *args: Any) -> Future[_T]: ...

[srittau]

Makes sense. PR welcome! There is also this comment in base_events.pyi:

typeshed/stdlib/3/asyncio/base_events.pyi

Lines 75 to 78 in e5dcf06

	# run_in_executor is defined as a coroutine in AbstractEventLoop but returns a Future in concrete implementation.
	# Need to ignore mypy Return type error as a result.
	def run_in_executor(self, executor: Any, # type: ignore
	func: Callable[..., _T], *args: Any) -> Future[_T]: ...

We should remove the comment and the corresponding # type: ignore as well.

[hauntsaninja]

Note your loop isn't an AbstractEventLoop, it's a BaseEventLoop which has the correct signature as Sebastian mentions. This was fixed in #3935, which chose not to make the change for AbstractEventLoop to remain true to CPython's implementation (https://github.com/python/cpython/blob/master/Lib/asyncio/events.py#L286). Since this change was pretty recent, you might still get errors with mypy.

[srittau]

I could have sworn that I checked AbstractEventLoop.run_in_executor() not to be an async function in the implementation. I think there is not much we can do in that case. asyncio.get_event_loop() is not guaranteed to return a loop implementation that returns a Future from run_in_executor(). The return value of get_event_loop() can be cast to the concrete loop implementation, which should work with the latest typeshed version.

[jamesba]

It seems like maybe the CPython implementation of AbstractEventLoop is a mistake on their part in that case then. I don't think it makes much sense for run_in_executor to be a coroutine function, since by its very nature it creates something that runs in another thread independently rather than something that needs to be awaited to make it execute.

[JelleZijlstra]

@jamesba I think you're right; sounds like we should report a bug to CPython.

[jamesba]

@JelleZijlstra agreed. Shall I report it?

[JelleZijlstra]

Please do.

[jamesba]

https://bugs.python.org/issue40782

[dgasmith]

It doesn't appear there is movement on the python issue tracker. Is there a way around this issue?

[gvanrossum]

I wonder if we need some way to indicate something that could either be a coroutine or a function returning a Future -- after all what you have to do with it is about the same (await it). But since we don't have that, fixing CPython sounds right. The fix does technically change behavior -- the coroutine version will only raise once you await it, while the Future-returning version will raise as soon as you call it. If that's problematic I suppose it could create a Future, set its exception, and return that Future.

Anyway, someone should just submit a PR to CPython.

[jamesba]

CPython PR now merged: python/cpython#21852

So I think we can now make the change here as well?

[srittau]

Sure, PR welcome! I don't think we need a version check here.