msg286011 - (view) |
Author: Jeremy Bustamante (Jeremy Bustamante) |
Date: 2017-01-22 14:09 |
Documemtation says sock_recv is a coroutine
https://docs.python.org/3.6/library/asyncio-eventloop.html#low-level-socket-operations
But following code says it isnt:
import asyncio
loop = asyncio.get_event_loop()
asyncio.iscoroutinefunction(loop.sock_recv)
False
asyncio.iscoroutine(loop.sock_recv)
False
|
msg286575 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2017-02-01 00:54 |
I'm not really familiar with asyncio terminology, but it looks like it returns a future:
>>> asyncio.isfuture(loop.sock_recv(sock, 1024))
True
There is a also a test in test_selector_events:
f = self.loop.sock_recv(sock, 1024)
self.assertIsInstance(f, asyncio.Future)
|
msg286677 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2017-02-01 19:10 |
OP is correct. sock_recv(), sock_sendall(), sock_connect(), sock_accept() all are functions that return Futures. The docs are wrong, as are the docstrings in the code.
|
msg286684 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2017-02-01 19:49 |
Here's a patch. sock_connect() wrapped by @coroutine in Lib/asyncio/selector_events.py so I left it as-is. Let me know if it still needs to be updated:
@coroutine
def sock_connect(self, sock, address):
"""Connect to a remote socket at address.
This method is a coroutine.
"""
...
return (yield from fut)
|
msg286689 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2017-02-01 20:06 |
I think the original idea was to document that all methods of the loop are coroutines, so that:
1. if a user needs a Future object they call ensure_future:
fut = asyncio.ensure_future(loop.method())
2. it gives us ability to refactor things. For instance, sock_connect was a method that returned Futures, but at one point of 3.5 we changed it to be coroutine. Because the method was documented as a coroutine, it wasn't strictly a backwards incompatible way.
In general, I think it would be safer for us to simple make all loop methods coroutines. Or, less radical, just keeping the status quo: document everything as a coroutine.
|
msg286704 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2017-02-01 21:32 |
The word coroutine has a more specific meaning though (and we have iscoroutine*() inspection functions to check for it).
Maybe we should switch all these to "awaitable"?
Also note that in proactor_events.py, sock_connect() is *not* a coroutine. In fact I'm not sure what it is -- it calls self._proactor.connect() which appears to return None from the code in windows_events.py. That's presumably a separate bug.
|
msg286707 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2017-02-01 21:46 |
> Maybe we should switch all these to "awaitable"?
I like this! Would it be OK to add a new sphinx declaration? So that:
.. coroutinemethod:: AbstractEventLoop.shutdown_asyncgens()
would become:
.. awaitable:: AbstractEventLoop.shutdown_asyncgens()
?
> Also note that in proactor_events.py, sock_connect() is *not* a coroutine.
Yeah, those small incompatibilities are inevitable for asyncio programs/frameworks -- something that returns a Future may occasionally become a coroutine. Your idea to document API methods as "awaitables" seems to be the right way to go.
> In fact I'm not sure what it is -- it calls self._proactor.connect() which appears to return None from the code in windows_events.py. That's presumably a separate bug.
It looks like it returns the result of "IocpProactor.connect()" call, which returns an _OverlappedFuture instance (I don't really know Windows part of asyncio code, so I might be missing something).
|
msg286710 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2017-02-01 21:58 |
A new Sphinx declaration sounds great to me, but you might want to check
with some Sphinx or Python-docs expert.
I somehow misread the code of IocpProactor.connect(), so ignore that part
-- the point is that it's not always a coroutine.
|
msg308808 - (view) |
Author: Andrew Svetlov (asvetlov) *  |
Date: 2017-12-20 20:38 |
sock_recv was converted into genuine coroutine in Python 3.7
Closing the issue.
|
msg310430 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2018-01-22 16:41 |
Andrew, shouldn't we fix 3.6 documentation? sock_* methods still documented as coroutines.
|
msg310431 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-01-22 16:49 |
> Andrew, shouldn't we fix 3.6 documentation? sock_* methods still documented as coroutines.
We only fixed it in 3.7 and the doc was updated. 3.6 stays as is.
|
msg310434 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2018-01-22 17:10 |
> We only fixed it in 3.7 and the doc was updated. 3.6 stays as is.
I'm not talking about fixing code in 3.6. In 3.6, sock_* methods still documented as coroutines but they are normal functions:
* https://docs.python.org/3.6/library/asyncio-eventloop.html#low-level-socket-operations
* https://github.com/python/cpython/blob/4002d5dbf4c058bbf2462f9f5dea057956d1caff/Doc/library/asyncio-eventloop.rst#low-level-socket-operations
* https://github.com/python/cpython/blob/4002d5dbf4c058bbf2462f9f5dea057956d1caff/Lib/asyncio/selector_events.py#L354
3.6 documentation still needs to be fixed. See my patch.
|
msg310435 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-01-22 17:16 |
Since we made them coroutines in 3.7, I'd prefer to keep the 3.6 documentation as is (so that people don't write code that expects them to be not coroutines in 3.6).
Please let's keep the current status quo.
|
msg310442 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2018-01-22 18:00 |
Well, the current status quo has already confused a user (see msg286011)
> (so that people don't write code that expects them to be not coroutines in 3.6)
Even if people expect these methods to be coroutines, two functions of the asyncio public API (iscoroutinefunction() and iscoroutine()) disagree with them as described in msg286011.
In similar scenarios, we've always keep the code as is and clarified the docs in stable releases, then changed the code behave as documented in master.
|
msg310445 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-01-22 18:17 |
They have been documented as coroutines and were returning futures since Python 3.4. I believe we can keep the status quo especially if only one user complained about it during the last 5 years.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:42 | admin | set | github: 73530 |
2018-01-22 18:17:06 | yselivanov | set | messages:
+ msg310445 |
2018-01-22 18:00:30 | berker.peksag | set | messages:
+ msg310442 |
2018-01-22 17:16:32 | yselivanov | set | messages:
+ msg310435 |
2018-01-22 17:10:13 | berker.peksag | set | messages:
+ msg310434 |
2018-01-22 16:49:02 | yselivanov | set | messages:
+ msg310431 |
2018-01-22 16:41:02 | berker.peksag | set | messages:
+ msg310430 |
2017-12-20 20:38:50 | asvetlov | set | status: open -> closed
versions:
- Python 3.5, Python 3.6 nosy:
+ asvetlov
messages:
+ msg308808 resolution: fixed stage: patch review -> resolved |
2017-02-01 21:58:49 | gvanrossum | set | messages:
+ msg286710 |
2017-02-01 21:46:57 | yselivanov | set | messages:
+ msg286707 |
2017-02-01 21:32:34 | gvanrossum | set | messages:
+ msg286704 |
2017-02-01 20:06:00 | yselivanov | set | messages:
+ msg286689 |
2017-02-01 19:49:24 | berker.peksag | set | files:
+ issue29344.diff
assignee: docs@python components:
+ Documentation
keywords:
+ patch nosy:
+ docs@python messages:
+ msg286684 stage: patch review |
2017-02-01 19:10:08 | gvanrossum | set | messages:
+ msg286677 versions:
+ Python 3.5, Python 3.7 |
2017-02-01 00:54:43 | berker.peksag | set | nosy:
+ berker.peksag messages:
+ msg286575
|
2017-01-22 14:09:45 | Jeremy Bustamante | create | |