Closed suyashkumar closed 3 years ago
Thanks for the well thought out comments on the draft! Responded inline, and also pushed some updates. Will take this PR out of draft now as well.
In the comments I will add a deprecated note on the existing iterator as well, and direct folks to use the stateful iterator, which will eventually become the default choice (pending some small benchmarks).
Sounds good!
Going to go ahead and aim to get this first piece submitted shortly.
This addresses the discussion in #183 by introducing a FlatStatefulIterator (that will be GC'd normally, even if it isn't exhausted). This also introduces ExhaustElementChannel which is an easy way for folks using the channel API to ensure it is always exhausted.
At some point either we should deprecate the channel-based API, or we should rename it so that it's not seen as the default for iteration (e.g. name it
FlatChannelIterator
or something). For most usecases, the stateful iterator should be sufficient.Future improvements can we made to the stateful iterator, for example not recomputing the flat element representation if the dataset hasn't changed from a previous call.