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

Type checking async iterator methods with mypy not working correctly #412

Closed
anriha opened this issue Aug 29, 2022 · 3 comments · Fixed by #413
Closed

Type checking async iterator methods with mypy not working correctly #412

anriha opened this issue Aug 29, 2022 · 3 comments · Fixed by #413
Labels
bug Something isn't working good first issue Good for newcomers small Low effort issue that can easily be picked up

Comments

@anriha
Copy link
Contributor

anriha commented Aug 29, 2022

When you try to type check method that returns stream mypy will complain about incompatible type with supertype. This is due to the fact that for mypy to consider something AsyncIterator it needs to have yield somewhere in it's implementation.

async def get_trucks(self, order_id: str) -> AsyncIterator["OrderTruckResponse"]:
        raise grpclib.GRPCError(grpclib.const.Status.UNIMPLEMENTED)

So something like this is considered Coroutine[Any, Any, AsyncIterator]], while

async def get_trucks(self, order_id: str) -> AsyncIterator["OrderTruckResponse"]:
        if False:
                yield OrderTruckRespnse()
        raise grpclib.GRPCError(grpclib.const.Status.UNIMPLEMENTED)

would be AsyncIterator["OrderTruckResponse"].

For more info check mypy issue.
python/mypy#12662

@Gobot1234 Gobot1234 added bug Something isn't working good first issue Good for newcomers small Low effort issue that can easily be picked up labels Aug 29, 2022
@Gobot1234
Copy link
Collaborator

I think the easiest way to do this is just

def fn():
    raise Error
    yield Type()

And that's a simple fix in the compiler and the jninja template

@anriha
Copy link
Contributor Author

anriha commented Aug 29, 2022

Yes that should definitely work and you are right that is better than my solution. If this solution is ok I don't mind opening PR with this.

@Gobot1234
Copy link
Collaborator

That'd be great thank you!

anriha added a commit to anriha/python-betterproto that referenced this issue Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers small Low effort issue that can easily be picked up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants