basedpyright icon indicating copy to clipboard operation
basedpyright copied to clipboard

inlay hints and generators

Open decorator-factory opened this issue 4 months ago • 5 comments

Code sample:

def foo():
    yield 420
    yield 69

async def bar():
    yield 420
    yield 69

f = bar
g = bar()

There are several issues with inlay hints and generator functions

Image
  1. When hovering or viewing/inserting the inlay hint, bar is incorrectly reported as returning a plain Generator instead of an AsyncGenerator. However, otherwise the type of bar and bar() seems to be inferred correctly.
  2. For both sync and async generators, inserting the inlay hint uses typing.Generator which is deprecated:
Image

decorator-factory avatar Oct 24 '25 14:10 decorator-factory

When hovering or viewing/inserting the inlay hint, bar is incorrectly reported as returning a plain Generator instead of an AsyncGenerator. However, otherwise the type of bar and bar() seems to be inferred correctly.

i never understood why you need to specify AsyncGenerator in the return type of an async generator function when the async keyword is already there. it's redundant information for the same reason that an async def function doesn't enforce that you wrap its return type in CoroutineType:

async def z() -> int:
    ...

reveal_type(z()) # CoroutineType[Any, Any, Literal[1]]

how annoying would it be if you had to do this instead:

async def z() -> CoroutineType[None, None, int]:
    ...

(sidenote: typescript does this and it's really annoying)

so i think the solution should be to just make it so that an async def doesn't need to have AsyncGenerator wrapping its return type. when i insert the inlay hint from your example, it only causes an error on the definition but the type in the usage is not effected, so i don't think changing this behavior will be a breaking change

For both sync and async generators, inserting the inlay hint uses typing.Generator which is deprecated:

#1356

DetachHead avatar Oct 24 '25 14:10 DetachHead

As these ships have long sailed, we can only accept their departure into our life... Maybe provide better diagnostics when it makes sense:

e.g.:

class Some(ABC):
    @abstractmethod
    async def gen(self) -> AsyncGenerator[int]:
        # 📎 It looks like you're trying to define an abstract async generator method!
        #    If the method body doesn't have a `yield` expression, you will need to
        #    make this a `def` function: as it stands, this defines a coroutine method
        #    at runtime instead.
        raise NotImplementedError

also see: https://discuss.python.org/t/overloads-of-async-generators-inconsistent-coroutine-wrapping/56665

decorator-factory avatar Oct 24 '25 15:10 decorator-factory

As these ships have long sailed, we can only accept their departure into our life

unless i'm missing something, i don't see why we need to accept that? sounds like a simple fix of just not reporting the error on the generator function definition

DetachHead avatar Oct 25 '25 04:10 DetachHead

Are you proposing to have a toggle that always wraps an async def function's return type in AsyncGenerator if it contains yield? Should this also be done with normal generator functions?

IMO it would be confusing to have different sets of semantics for your own code and interpreting types in library code (e.g.: if you vendor a library or stubs for a library, do you manually change all the -> Iterator and -> Generator return types?). The same reason it's difficult to switch float to not mean float | int

(that's opinionated of course, maybe there are users for whom this doesn't break anything but who would like the reduced noise in annotations)

decorator-factory avatar Oct 25 '25 06:10 decorator-factory

yeah i guess it does risk making the situation even more confusing

DetachHead avatar Oct 25 '25 09:10 DetachHead