sam-mosleh · GitHub

Aha! Thanks for the finding :) Them I'm good with changing to Awaitable. I'd like to have a new unit test though that explicitly check that Future is supported by Application.create_task. Could you add that to test_application.py? Please also add to the Note in the docs of Application.create_task a bullet points that clarifies that support for Future is offered in addition to the functionality of asyncio.create_task.

Moreover, I realize that Application.create_task does in fact not support generators - in contrast to asyncio.create_task. This can be fixed by changing return await coroutine to return await asyncio.create_task(coroutine) in __create_task_callback. That way, Application.create_task would fully cover the functionality of asyncio.gather plus also supporting Future.

Would you like to add this to your PR? this would include

  • A unit test that makes sure that generators work
  • Updating the type hints & docs of Application.create_task accordingly

If you'd rather like to keep your PR as-is, that's perfectly fine. It can go in a follow-up PR, then.

PS: the type completeness check can't cope with PRs from external repos yet. before merging we should check the type completeness manually

Read the original on github.com ↗