pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

feat: add `coroutine::await_in_coroutine` to await awaitables in coroutine context

Open wyfo opened this issue 7 months ago • 5 comments

Relates to #1632

Draft based on #3610

wyfo avatar Nov 30 '23 15:11 wyfo

CodSpeed Performance Report

Merging #3611 will not alter performance

Comparing wyfo:pyfuture (18f59ea) with main (6c2e6f8)

Summary

✅ 69 untouched benchmarks

codspeed-hq[bot] avatar Nov 30 '23 16:11 codspeed-hq[bot]

@wyfo I'm sorry for the painfully slow review here.

Given that allow_threads is currently under intense scrutiny and we haven't yet decided on a concrete solution, #3610 risks getting stuck for a bit while we figure out a way forward for that API.

Does this patch rely on #3610 getting merged, or is it possible to rebase this on main? That would allow us to move forward with some of these remaining async PRs.

davidhewitt avatar Dec 17 '23 08:12 davidhewitt

There is one thing bothering me with this implementation: the name PyFuture. In fact, there is Future object in Python, and it doesn't match the underlying type of PyFuture. PyFuture is the result of calling __await__, so it's an iterator, but that's all we know, and I don't know any naming convention for this object – PEP 492 doesn't give it a name.

I've chosen PyFuture because it sounds like Rust Future, but that's maybe not a good reason. If you have some name suggestion, I'm interested.

wyfo avatar Apr 07 '24 02:04 wyfo

The error in CI (other than non_local_definition lint) is due to the bug mentioned in #4055.

wyfo avatar Apr 07 '24 11:04 wyfo

As written above, the name PyFuture was not a good name (the Python object doesn't even have a name, see this discussion), so I dropped it here to reuse it in #4057 where it's more suited. Instead, I chose the more explicit name await_in_coroutine, making it easier to document, and for the user to understand that it must be used in coroutine context.

wyfo avatar May 07 '24 08:05 wyfo