encode / broadcaster

Broadcast channels for async web apps. 📢
BSD 3-Clause "New" or "Revised" License
1.13k stars 121 forks source link

Fix type hint on `Subscriber.__aiter__` #136

Closed wookie184 closed 3 months ago

wookie184 commented 3 months ago

I've read the contributing guidelines and believe this is clear enough to open an issue directly. I might open a PR at some point but if someone else can get to it first please do!

Current type hint: https://github.com/encode/broadcaster/blob/23c9b40136231d0a78d1299588b62d7162067ff4/broadcaster/_base.py#L113 Incorrect complaint by pyright cause by this: image

Correct type hint:

async def __aiter__(self) -> AsyncGenerator[Event, None]: 

Or the also correct and a bit nicer (and consistent with Broadcast.subscribe):

async def __aiter__(self) -> AsyncIterator[Event]:
alex-oleshkevich commented 3 months ago

It is a generator because it yield's.

wookie184 commented 3 months ago

collections.abc.AsyncGenerator inherits from collections.abc.AsyncIterator, so you can use the more generic AsyncIterator if you aren't using any generator-specific functionality (like sending values to the generator). This is mentioned in the typing docs here.

I mentioned it because it's already used for the async generator a few lines above. Either option is fine though: https://github.com/encode/broadcaster/blob/23c9b40136231d0a78d1299588b62d7162067ff4/broadcaster/_base.py#L90

alexandreseris commented 3 months ago

If you prefer keep the generator type, I think you can just change the return type of Subscriber.__aiter__ to AsyncGenerator[Event, None], since __iter__ never return None and its value are also never None but plain Event.

That should resolve the error wookie184 described

alex-oleshkevich commented 3 months ago

Merged.