experimental-inspect: Iterator __next__ not optional - #6274
Conversation
| /// The Python type an iterator's `__next__` produces. | ||
| /// | ||
| /// PyO3 lets `__next__` be written as `-> Option<T>` (or `-> PyResult<Option<T>>`), where `None` | ||
| /// means "raise `StopIteration`" rather than "yield `None`". At the Python level that is not a | ||
| /// returned value at all, so the stub has to say `T`: a stub saying `T | None` makes the loop | ||
| /// variable optional and stops the class satisfying `Iterator[T]`. | ||
| /// | ||
| /// The unwrapping is done with an inherent const on the concrete shapes, which takes priority over | ||
| /// the trait const used for everything else. Everything that is not an `Option` — an iterator whose | ||
| /// `__next__` always yields and signals exhaustion by raising — falls through to | ||
| /// [`PyReturnType`] unchanged. | ||
| pub struct IterNextOutput<T>(PhantomData<T>); | ||
|
|
||
| /// The fallback used when the return type is not an `Option`. | ||
| pub trait IterNextOutputFallback: iter_next::Sealed { | ||
| /// The function return type | ||
| const OUTPUT_TYPE: PyStaticExpr; | ||
| } | ||
|
|
||
| mod iter_next { | ||
| pub trait Sealed {} | ||
| impl<T> Sealed for super::IterNextOutput<T> {} | ||
| } |
There was a problem hiding this comment.
It seems unfortunate that this machinery is not built on the same structures handling IterNextOutput in impl_/pymethods.rs
Having it build in the same machinery would avoid any accidental future divergence. We might need to move that machinery to use const-generic specialization, however. Not sure.
Comparing to that machinery immediately shows that __anext__ has the same problem.
|
Hey @davidhewitt, thanks for your review! Unfortunately I won't have much time until beginning of next week, and I also am not enough of an expert yet in this codebase to accurately judge with which alternative implementation to proceed. The best I came up with is the following exploration:
Then, there are 3 different solutions I explored branching of the test harness above:
Solutions A & B clear up some code duplication, while C basically keeps existing things as-is. => If this input is helpful, you could tell me which direction looks promising. Also feel free to commit and/or use this PR as much as you like, if needed (I saw you wanted to have this included in the next release). If you don't find time and above commits understandibly are a bit too much to review, I could have a more thorough and concentrated look next week! |
|
I prefer solution A please |
…anext__` shapes Route A: one wrapper carrying both the conversion and the type hint Review pass on the `IterNextOutput` wrapper pytests: assert the generated `__next__` / `__anext__` hints
3741b0e to
ffad12d
Compare
|
Implemented the test harness + solution A |
2 participants
PyO3 lets
__next__be written as-> Option<T>, whereNonemeans "raiseStopIteration". Introspection resolved that through the ordinary return-type path, so the stub said:Which is wrong in a way that costs more than
Incompletewould. mypy takes__next__'s return type as the iteration item type, so everyfor x in itbindsint | None, and the class stops satisfyingIterator[int].