Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add SyncQueue and AsyncQueue Protocols #374

Merged

Conversation

@rbuffat
Copy link
Contributor

@rbuffat rbuffat commented Nov 4, 2021

What do these changes do?

Add protocols to avoid the use of private classes _SyncQueueProxy and _AsyncQueueProxy when adding type hints.

Are there changes in behavior for the user?

I assume that not. But it would be good if somebody more experienced could check this.

Related issue number

See #372

This PR is not intended to be directly merged, but to open a discussion about this approach.

Using this approach, the following seems to work for adding type hints:

import asyncio
import janus


def threaded(sync_q: janus.SyncQueue[int]):
    for i in range(100):
        sync_q.put(i)
    sync_q.join()


async def async_coro(async_q: janus.AsyncQueue[int]):
    for i in range(100):
        val = await async_q.get()
        assert val == i
        async_q.task_done()


async def main():
    queue: janus.Queue[int] = janus.Queue()
    loop = asyncio.get_running_loop()
    fut = loop.run_in_executor(None, threaded, queue.sync_q)
    await async_coro(queue.async_q)
    await fut
    queue.close()
    await queue.wait_closed()


asyncio.run(main())

Full disclaimer: I never used Protocols before. I skimmed https://www.python.org/dev/peps/pep-0544 and watched https://www.youtube.com/watch?v=kDDCKwP7QgQ
Also, I only recently started using typing with Python.

While this approach seems to work, it looks rather verbose.

@rbuffat
Copy link
Contributor Author

@rbuffat rbuffat commented Nov 4, 2021

The Protocol base class is provided in the typing_extensions package for Python 2.7 and 3.4-3.7. Starting with Python 3.8, Protocol is included in the typing module.

https://mypy.readthedocs.io/en/stable/protocols.html

Copy link
Member

@asvetlov asvetlov left a comment

The proposed solution is a little verbose but technically should work perfectly fine.

Please apply a note about typing_extensions before the PR merging.

@@ -7,7 +7,7 @@
from heapq import heappop, heappush
from queue import Empty as SyncQueueEmpty
from queue import Full as SyncQueueFull
from typing import Any, Callable, Deque, Generic, List, Optional, Set, TypeVar
from typing import Any, Callable, Deque, Generic, List, Optional, Protocol, Set, TypeVar
Copy link
Member

@asvetlov asvetlov Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please import Protocol from typing_extensions module; add typing-extensions>=3.7.4.3 here

@rbuffat rbuffat force-pushed the experiment_with_protocols_for_typing_queues branch from 610d860 to 03856ab Nov 24, 2021
@rbuffat rbuffat force-pushed the experiment_with_protocols_for_typing_queues branch from 03856ab to 3360844 Nov 24, 2021
@asvetlov asvetlov merged commit 5389dec into aio-libs:master Nov 24, 2021
3 checks passed
@asvetlov
Copy link
Member

@asvetlov asvetlov commented Nov 24, 2021

Thanks!

@asvetlov
Copy link
Member

@asvetlov asvetlov commented Nov 24, 2021

0.7.0 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants