Allow non-ExactSizeIterator in PyList::new#5830
Conversation
bschoenmaeckers
force-pushed
the
list-new-not-exact-size
branch
from
eae5df8 to
c87e2c3
Compare
src/types/list.rs
Outdated
| } | ||
| } | ||
|
|
||
| impl ExactSizeIterator for FaultyIter { |
There was a problem hiding this comment.
I guess we can just drop this impl?
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, this seems like a positive improvement to me, I remember wanting it in the past.
I just checked what Vec does when extending and it looks like it'll try to reserve at least the lower bound, so for cases where we've exceeded the iterator lower bound it'll just be equivalent to pushing element-by-element and resizing when needed, like here.
bschoenmaeckers
force-pushed
the
list-new-not-exact-size
branch
from
b8db69e to
e529a54
Compare
| Ok::<_, PyErr>(count + 1) | ||
| })?; | ||
|
|
||
| assert_eq!(len, count, "Attempted to create PyList but `elements` was smaller than reported by its `size_hint` implementation."); |
There was a problem hiding this comment.
I'm still trying to decide about this assertion. On one hand, if users have bad iterator implementations, this panic will help them clean up.
On the other hand, I still think it might be user-friendly to just truncate the list to count length.
Maybe for now we just document that we will panic if the iterator passes fewer elements than the size hint (and that we may change that in the future)?
There was a problem hiding this comment.
Message about the panic should already be in the docs of this function
3 participants
As discussed in #5822
xref #4782