fix: improper types in pagers generation

[dpcollins-google]

Generators are Iterators not Iterables https://docs.python.org/3/library/typing.html#typing.Generator

[tseaver]

The change is correct, if a bit nit-picky (Iterable is a less restrictive contract than Iterator, and a strict superset). Do we have cases where type-checking actually complains about the difference? I guess that would be clients calling next(thing) directly?

[dpcollins-google]

The change is correct, if a bit nit-picky (Iterable is a less restrictive contract than Iterator, and a strict superset). Do we have cases where type-checking actually complains about the difference? I guess that would be clients calling next(thing) directly?

This is not quite correct. Iterables are not required to have __next__ methods, they only usually do :) https://docs.python.org/3/library/collections.abc.html#collections.abc.Iterable

In fact, you can see this with <*>Pager itself: it is an Iterable (with __iter__) but not an Iterator (no __next__)

[busunkim96]

@dpcollins-google could you update the golden files? That should resolve the one failing Tests / integration check. See instructions here.

[tseaver]

@dpcollins-google I think we are in violent agreement here: Iterable requires only __iter__, while Iterator must also have __next__ which is a) more restrictive than Iterable, and b) makes the contract for Iterator a "sub-type" of Iterable (no types which implement Iterator do not also implement Iterable).

At any rate, my question remains: do we have reported errors from pytype in this repo, or even in the wild, where declaring the less-restrictive type (Iterable) is causing errors? My surmise is that either in this repo or elsewhere, somebody is using next(response), rather than for foo in response:.

[busunkim96]

It looks like this is causing a problem for Pub/Sub (see log, so I'm inclined to merge it now and re-visit if it causes an issue.

@software-dov @atulep Could you also take a look as the generator owners?

[dpcollins-google]

To clarify, the issue this is causing is that PyType detects that its wrong: the pager object itself is NOT an Iterator because its __iter__ method doesn't return an Iterator, it returns an Iterable, which doesn't have a __next__ method defined.

[atulep]

From https://docs.python.org/3/library/typing.html#typing.Generator, generator can have a return type of either Iterable or Iterator. Why is this change needed?

Does Pub/Sub fail because a caller of the pager expects an iterator instead of iterable?

[dpcollins-google]

Please see my comment above. The Pager itself does not model Iterable because its __iter__ method states incorrectly that it returns an Iterable when it needs to return an Iterator.