Implement itertools.zip_longest #1604
Conversation
vm/src/stdlib/itertools.rs
Outdated
| for idx in 0..self.iterators.len() { | ||
| next_obj = match call_next(vm, &self.iterators[idx]) { | ||
| Ok(obj) => obj, | ||
| Err(_) => { |
There was a problem hiding this comment.
You might want to check if the error here is a StopIteration or not and handle it accordingly (if it isn't StopIteration we just want to pass it on):
Err(err) => {
if !objtype::isinstance(&err, &vm.ctx.exceptions.stop_iteration) {
return Err(err);
}
numactive -= 1;
// ...
}
vm/src/stdlib/itertools.rs
Outdated
| struct PyItertoolsZiplongest { | ||
| iterators: Vec<PyObjectRef>, | ||
| fillvalue: PyObjectRef, | ||
| numactive: RefCell<usize>, |
There was a problem hiding this comment.
This can just be a Cell<usize>. Cell is better for primitive data types that can just be copied. Then, later, you can just do let numactive = self.numactive.get(); and self.numactive.set(new_numactive);
vm/src/stdlib/itertools.rs
Outdated
| let mut numactive = self.numactive.clone().into_inner(); | ||
|
|
||
| for idx in 0..self.iterators.len() { | ||
| next_obj = match call_next(vm, &self.iterators[idx]) { |
There was a problem hiding this comment.
Do you need to have next_obj be mutable and declared outside the loop? I think you could just do let next_obj = match call_next ....
|
@coolreader18 Thank you for your detailed review! I learned a lot from this review. |
| Err(new_stop_iteration(vm)) | ||
| } else { | ||
| let mut result: Vec<PyObjectRef> = Vec::new(); | ||
| let mut numactive = self.numactive.get(); |
There was a problem hiding this comment.
Does self.numactive actually need to be a Cell? It doesn't look like you set it anywhere, but I'm not sure if it should be set at some point or if it doesn't need to be.
| let mut result: Vec<PyObjectRef> = Vec::new(); | ||
| let mut numactive = self.numactive.get(); | ||
|
|
||
| for idx in 0..self.iterators.len() { |
There was a problem hiding this comment.
You can make this for iter in &self.iterators { ... }
|
Thanks for contributing! |
This pull request implements
itertools.zip_longestin Python standard library, related with #1361.