python / cpython

The Python programming language
https://www.python.org
Other
63.36k stars 30.35k forks source link

`None` vs `Never` as `typing.Generator`'s send type #123521

Open sterliakov opened 2 months ago

sterliakov commented 2 months ago

Documentation

Documentation for typing.Generator currently says:

If your generator will only yield values, set the SendType and ReturnType to None

While not obviously wrong, using None as SendType looks imprecise: we want a typechecker to warn about .send(t) calls for any t, right? Using that method likely indicates that the generator is used incorrectly.

Now type checkers will reject .send(t) calls for all t types except None and Any.

Since python 3.11 we have a Never type - effectively a "please don't" type. So setting SendType to Never instead of None would be more useful from the interface declaration perspective: literally saying "please don't send anything here".

I do understand that None is there to match runtime behaviour closer since calling it.send(None) is equivalent to calling next(it) for the next time, so None is formally correct. However, there's no benefit ever from doing it.send(None) if the generator does not use sent values - calling next(it) is clearly preferable.

My suggested solution is to recommend typing.Never as SendType in such cases in documentation. I'm ready to write a PR with this change.

ZeroIntensity commented 2 months ago

typing.Generator is deprecated in favor of collections.abc.Generator, maybe this section should be moved to over there (as well as the fix)?

sterliakov commented 2 months ago

Hm, it's an interesting question that needs a separate ticket, IMO. I'll create and link it here.

Your observation applies to other such generics - Callable, Coroutine and AsyncGenerator also have typing-related descriptions there that aren't present in corresponding collections.abc documentation entries.

I'm also not sure whether it should be moved or duplicated there - obviously moving is saner, but harms searching significantly.

ZeroIntensity commented 2 months ago

IMO, if we want to people to migrate to collections.abc, having the only examples available on the typing page is probably a bad idea. I think we should move them.

zuo commented 1 month ago

I do understand that None is there to match runtime behaviour closer since calling it.send(None) is equivalent to calling next(it) for the next time, so None is formally correct. However, there's no benefit ever from doing it.send(None) if the generator does not use sent values - calling next(it) is clearly preferable.

Typically, next(it) is preferable for such a generator, but it.send(None) is still perfectly valid, and in some cases it may be more convenient, e.g., just to treat various kinds of generators uniformly in some code...

I'd say that the generator's interface should not try to know better the context in which it may be used. That's the competence of the client code (the code which makes use of that interface).

TeamSpen210 commented 1 month ago

To me, it makes sense to change send() to accept SendType | None) - regardless of what SendType is, None is an acceptable call. Then using SendType = Never would be correct, the union simplifies to just None. Preventing send(None) isn't really too useful, since the caller could just as easily do next(), causing the same unsafety. The problem really is that regardless of the defined send type, generators always need None sent the first time, but only then.

If you do want to protect against that, you'd really need some sort of wrapper - maybe a function which calls the generator, advances once, then returns it casted to a protocol that disallows iteration/next.