Skip to content

Commit

Permalink
Add PyAnyMethods.try_iter and deprecate .iter
Browse files Browse the repository at this point in the history
Fixes PyO3#4550.
  • Loading branch information
LilyFoote committed Sep 13, 2024
1 parent d14bfd0 commit bb88e06
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 19 deletions.
1 change: 1 addition & 0 deletions newsfragments/4550.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate `PyAnyMethods.iter` and replace it with `PyAnyMethods.try_iter`
2 changes: 1 addition & 1 deletion pyo3-benches/benches/bench_any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ fn bench_collect_generic_iterator(b: &mut Bencher<'_>) {

b.iter(|| {
collection
.iter()
.try_iter()
.unwrap()
.collect::<PyResult<Vec<_>>>()
.unwrap()
Expand Down
2 changes: 1 addition & 1 deletion src/conversions/smallvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ where
};

let mut sv = SmallVec::with_capacity(seq.len().unwrap_or(0));
for item in seq.iter()? {
for item in seq.try_iter()? {
sv.push(item?.extract::<A::Item>()?);
}
Ok(sv)
Expand Down
13 changes: 12 additions & 1 deletion src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,13 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed {
///
/// This is typically a new iterator but if the argument is an iterator,
/// this returns itself.
fn try_iter(&self) -> PyResult<Bound<'py, PyIterator>>;

/// Takes an object and returns an iterator for it.
///
/// This is typically a new iterator but if the argument is an iterator,
/// this returns itself.
#[deprecated(since = "0.23.0", note = "use `try_iter` instead")]
fn iter(&self) -> PyResult<Bound<'py, PyIterator>>;

/// Returns the Python type object for this object's type.
Expand Down Expand Up @@ -1381,10 +1388,14 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
)
}

fn iter(&self) -> PyResult<Bound<'py, PyIterator>> {
fn try_iter(&self) -> PyResult<Bound<'py, PyIterator>> {
PyIterator::from_object(self)
}

fn iter(&self) -> PyResult<Bound<'py, PyIterator>> {
self.try_iter()
}

fn get_type(&self) -> Bound<'py, PyType> {
unsafe { PyType::from_borrowed_type_ptr(self.py(), ffi::Py_TYPE(self.as_ptr())) }
}
Expand Down
12 changes: 6 additions & 6 deletions src/types/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{ffi, Bound, PyAny, PyErr, PyResult, PyTypeCheck};
/// Python::with_gil(|py| -> PyResult<()> {
/// let list = py.eval(c_str!("iter([1, 2, 3, 4])"), None, None)?;
/// let numbers: PyResult<Vec<usize>> = list
/// .iter()?
/// .try_iter()?
/// .map(|i| i.and_then(|i|i.extract::<usize>()))
/// .collect();
/// let sum: usize = numbers?.iter().sum();
Expand Down Expand Up @@ -115,7 +115,7 @@ mod tests {
Python::with_gil(|py| {
let obj = vec![10, 20].to_object(py);
let inst = obj.bind(py);
let mut it = inst.iter().unwrap();
let mut it = inst.try_iter().unwrap();
assert_eq!(
10_i32,
it.next().unwrap().unwrap().extract::<'_, i32>().unwrap()
Expand All @@ -138,7 +138,7 @@ mod tests {

Python::with_gil(|py| {
let inst = obj.bind(py);
let mut it = inst.iter().unwrap();
let mut it = inst.try_iter().unwrap();

assert_eq!(
10_i32,
Expand Down Expand Up @@ -166,7 +166,7 @@ mod tests {

{
let inst = list.bind(py);
let mut it = inst.iter().unwrap();
let mut it = inst.try_iter().unwrap();

assert_eq!(
10_i32,
Expand Down Expand Up @@ -199,7 +199,7 @@ def fibonacci(target):
let generator = py
.eval(ffi::c_str!("fibonacci(5)"), None, Some(&context))
.unwrap();
for (actual, expected) in generator.iter().unwrap().zip(&[1, 1, 2, 3, 5]) {
for (actual, expected) in generator.try_iter().unwrap().zip(&[1, 1, 2, 3, 5]) {
let actual = actual.unwrap().extract::<usize>().unwrap();
assert_eq!(actual, *expected)
}
Expand Down Expand Up @@ -327,7 +327,7 @@ def fibonacci(target):
fn length_hint_becomes_size_hint_lower_bound() {
Python::with_gil(|py| {
let list = py.eval(ffi::c_str!("[1, 2, 3]"), None, None).unwrap();
let iter = list.iter().unwrap();
let iter = list.try_iter().unwrap();
let hint = iter.size_hint();
assert_eq!(hint, (3, None));
});
Expand Down
6 changes: 3 additions & 3 deletions src/types/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ mod tests {
// Can't just compare against a vector of tuples since we don't have a guaranteed ordering.
let mut key_sum = 0;
let mut value_sum = 0;
for el in mapping.items().unwrap().iter().unwrap() {
for el in mapping.items().unwrap().try_iter().unwrap() {
let tuple = el.unwrap().downcast_into::<PyTuple>().unwrap();
key_sum += tuple.get_item(0).unwrap().extract::<i32>().unwrap();
value_sum += tuple.get_item(1).unwrap().extract::<i32>().unwrap();
Expand All @@ -311,7 +311,7 @@ mod tests {
let mapping = ob.downcast::<PyMapping>().unwrap();
// Can't just compare against a vector of tuples since we don't have a guaranteed ordering.
let mut key_sum = 0;
for el in mapping.keys().unwrap().iter().unwrap() {
for el in mapping.keys().unwrap().try_iter().unwrap() {
key_sum += el.unwrap().extract::<i32>().unwrap();
}
assert_eq!(7 + 8 + 9, key_sum);
Expand All @@ -329,7 +329,7 @@ mod tests {
let mapping = ob.downcast::<PyMapping>().unwrap();
// Can't just compare against a vector of tuples since we don't have a guaranteed ordering.
let mut values_sum = 0;
for el in mapping.values().unwrap().iter().unwrap() {
for el in mapping.values().unwrap().try_iter().unwrap() {
values_sum += el.unwrap().extract::<i32>().unwrap();
}
assert_eq!(32 + 42 + 123, values_sum);
Expand Down
10 changes: 5 additions & 5 deletions src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ where
};

let mut v = Vec::with_capacity(seq.len().unwrap_or(0));
for item in seq.iter()? {
for item in seq.try_iter()? {
v.push(item?.extract::<T>()?);
}
Ok(v)
Expand Down Expand Up @@ -614,7 +614,7 @@ mod tests {
let ob = v.to_object(py);
let seq = ob.downcast_bound::<PySequence>(py).unwrap();
let mut idx = 0;
for el in seq.iter().unwrap() {
for el in seq.try_iter().unwrap() {
assert_eq!(v[idx], el.unwrap().extract::<i32>().unwrap());
idx += 1;
}
Expand Down Expand Up @@ -646,7 +646,7 @@ mod tests {
let concat_seq = seq.concat(seq).unwrap();
assert_eq!(6, concat_seq.len().unwrap());
let concat_v: Vec<i32> = vec![1, 2, 3, 1, 2, 3];
for (el, cc) in concat_seq.iter().unwrap().zip(concat_v) {
for (el, cc) in concat_seq.try_iter().unwrap().zip(concat_v) {
assert_eq!(cc, el.unwrap().extract::<i32>().unwrap());
}
});
Expand All @@ -661,7 +661,7 @@ mod tests {
let concat_seq = seq.concat(seq).unwrap();
assert_eq!(12, concat_seq.len().unwrap());
let concat_v = "stringstring".to_owned();
for (el, cc) in seq.iter().unwrap().zip(concat_v.chars()) {
for (el, cc) in seq.try_iter().unwrap().zip(concat_v.chars()) {
assert_eq!(cc, el.unwrap().extract::<char>().unwrap());
}
});
Expand All @@ -676,7 +676,7 @@ mod tests {
let repeat_seq = seq.repeat(3).unwrap();
assert_eq!(6, repeat_seq.len().unwrap());
let repeated = ["foo", "bar", "foo", "bar", "foo", "bar"];
for (el, rpt) in repeat_seq.iter().unwrap().zip(repeated.iter()) {
for (el, rpt) in repeat_seq.try_iter().unwrap().zip(repeated.iter()) {
assert_eq!(*rpt, el.unwrap().extract::<String>().unwrap());
}
});
Expand Down
4 changes: 2 additions & 2 deletions tests/test_proto_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ impl DefaultedContains {
fn __iter__(&self, py: Python<'_>) -> PyObject {
PyList::new(py, ["a", "b", "c"])
.as_ref()
.iter()
.try_iter()
.unwrap()
.into()
}
Expand All @@ -866,7 +866,7 @@ impl NoContains {
fn __iter__(&self, py: Python<'_>) -> PyObject {
PyList::new(py, ["a", "b", "c"])
.as_ref()
.iter()
.try_iter()
.unwrap()
.into()
}
Expand Down

0 comments on commit bb88e06

Please sign in to comment.