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

Fix type hint on Subscriber.__aiter__ #136

Closed
wookie184 opened this issue Aug 7, 2024 · 4 comments
Closed

Fix type hint on Subscriber.__aiter__ #136

wookie184 opened this issue Aug 7, 2024 · 4 comments

Comments

@wookie184
Copy link

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:

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

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
Copy link
Member

It is a generator because it yield's.

@wookie184
Copy link
Author

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:

async def subscribe(self, channel: str) -> AsyncIterator[Subscriber]:

@alexandreseris
Copy link

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 added a commit that referenced this issue Aug 23, 2024
@alex-oleshkevich
Copy link
Member

Merged.

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

No branches or pull requests

3 participants