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

Propagate the return type #145

Merged
merged 7 commits into from
Jul 16, 2024
Merged

Conversation

dkang-quora
Copy link
Contributor

Test code:

import asyncio
from typing import Any, Generator

from asynq import asynq


@asynq()
def noop():
    return


# case: no annotation + no yield
@asynq()
def f1():
    return 100


# case: no annotation + yield
@asynq()
def f2():
    yield noop.asynq()
    return 100


# case: annotation + no yield
@asynq()
def f3() -> int:
    return 300


# case: annotation + yield
@asynq()
def f4() -> Generator[Any, Any, int]:
    yield noop()
    return 300


async def main():
    _ret1: int = await f1.asyncio()
    _ret2: str = await f1.asyncio()  # "int" is incompatible with "str" (reportAssignmentType)

    _ret1: int = await f2.asyncio()
    _ret2: str = await f2.asyncio()  # "Literal[100]" is incompatible with "str" (reportAssignmentType)

    _ret1: int = await f3.asyncio()
    _ret2: str = await f3.asyncio()  # "int" is incompatible with "str" (reportAssignmentType)

    _ret1: int = await f4.asyncio()
    _ret2: str = await f4.asyncio()  # "int" is incompatible with "str" (reportAssignmentType)


asyncio.run(main())
  • if we add asynq.G = Generator[Any, Any, T] for convenience and have pyanalyze accept both T and G[T], the existing asynq codebase can be converted to fully compatible code with pyright.
@asynq()
def f() -> G[int]:
  yield op.asynq()
  return 100
  • Using TypeAlias(_G, _Coroutine, _CoroutineFn) did not work with pyright. I expanded them.
  • The PR can wrongly unwrap Generator[, , T] as an edge case as I shared in the previous PR, but it's better than nothing

@JelleZijlstra
Copy link
Collaborator

Could you add a test case to the repo like in #144? These types are tricky enough that I'd like CI coverage to make sure it doesn't break. Or maybe we can merge #144 first and add this on top of that change.

@JelleZijlstra
Copy link
Collaborator

Internal testing shows that our current types break when methods are involved, I'll push something to try to address that.

@JelleZijlstra
Copy link
Collaborator

I removed the attempts to save the return type for now; they appear to be causing problems with generic functions. I'll merge this first to unblock internal use cases that are currently blocked, then try again to get return types to propagate.

@JelleZijlstra JelleZijlstra merged commit 4cdc7a6 into quora:master Jul 16, 2024
8 of 9 checks passed
@dkang-quora dkang-quora deleted the return-type branch July 16, 2024 22:20
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

Successfully merging this pull request may close these issues.

2 participants