From 16a624d2e42032373fc60602b9453ef9545b402c Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Fri, 4 Oct 2024 18:06:57 +0100 Subject: [PATCH] Add PyAnyMethods.try_iter and deprecate .iter (#4553) * Add PyAnyMethods.try_iter and deprecate .iter Fixes #4550. * Rename newsfragment to use PR number * Add example to try_iter docs --- newsfragments/4553.fixed.md | 1 + pyo3-benches/benches/bench_any.rs | 2 +- src/conversions/smallvec.rs | 2 +- src/types/any.rs | 33 ++++++++++++++++++++++++++++++- src/types/iterator.rs | 12 +++++------ src/types/mapping.rs | 6 +++--- src/types/sequence.rs | 10 +++++----- tests/test_proto_methods.rs | 4 ++-- 8 files changed, 51 insertions(+), 19 deletions(-) create mode 100644 newsfragments/4553.fixed.md diff --git a/newsfragments/4553.fixed.md b/newsfragments/4553.fixed.md new file mode 100644 index 00000000000..c6714bfce95 --- /dev/null +++ b/newsfragments/4553.fixed.md @@ -0,0 +1 @@ +Deprecate `PyAnyMethods.iter` and replace it with `PyAnyMethods.try_iter` diff --git a/pyo3-benches/benches/bench_any.rs b/pyo3-benches/benches/bench_any.rs index af2d226b3aa..4ed14493873 100644 --- a/pyo3-benches/benches/bench_any.rs +++ b/pyo3-benches/benches/bench_any.rs @@ -77,7 +77,7 @@ fn bench_collect_generic_iterator(b: &mut Bencher<'_>) { b.iter(|| { collection - .iter() + .try_iter() .unwrap() .collect::>>() .unwrap() diff --git a/src/conversions/smallvec.rs b/src/conversions/smallvec.rs index be90113344e..b9e48ace2dd 100644 --- a/src/conversions/smallvec.rs +++ b/src/conversions/smallvec.rs @@ -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::()?); } Ok(sv) diff --git a/src/types/any.rs b/src/types/any.rs index 409630e12b2..76c5bcf18dd 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -659,10 +659,37 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { where K: IntoPyObject<'py>; + /// Takes an object and returns an iterator for it. Returns an error if the object is not + /// iterable. + /// + /// This is typically a new iterator but if the argument is an iterator, + /// this returns itself. + /// + /// # Example: Checking a Python object for iterability + /// + /// ```rust + /// use pyo3::prelude::*; + /// use pyo3::types::{PyAny, PyNone}; + /// + /// fn is_iterable(obj: &Bound<'_, PyAny>) -> bool { + /// match obj.try_iter() { + /// Ok(_) => true, + /// Err(_) => false, + /// } + /// } + /// + /// Python::with_gil(|py| { + /// assert!(is_iterable(&vec![1, 2, 3].into_pyobject(py).unwrap())); + /// assert!(!is_iterable(&PyNone::get(py))); + /// }); + /// ``` + fn try_iter(&self) -> PyResult>; + /// 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>; /// Returns the Python type object for this object's type. @@ -1381,10 +1408,14 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { ) } - fn iter(&self) -> PyResult> { + fn try_iter(&self) -> PyResult> { PyIterator::from_object(self) } + fn iter(&self) -> PyResult> { + self.try_iter() + } + fn get_type(&self) -> Bound<'py, PyType> { unsafe { PyType::from_borrowed_type_ptr(self.py(), ffi::Py_TYPE(self.as_ptr())) } } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 788ed79c475..b86590e5de8 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -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> = list -/// .iter()? +/// .try_iter()? /// .map(|i| i.and_then(|i|i.extract::())) /// .collect(); /// let sum: usize = numbers?.iter().sum(); @@ -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() @@ -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, @@ -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, @@ -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::().unwrap(); assert_eq!(actual, *expected) } @@ -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)); }); diff --git a/src/types/mapping.rs b/src/types/mapping.rs index 5ad1aa1b3c1..6249b0eb97b 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -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::().unwrap(); key_sum += tuple.get_item(0).unwrap().extract::().unwrap(); value_sum += tuple.get_item(1).unwrap().extract::().unwrap(); @@ -311,7 +311,7 @@ mod tests { let mapping = ob.downcast::().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::().unwrap(); } assert_eq!(7 + 8 + 9, key_sum); @@ -329,7 +329,7 @@ mod tests { let mapping = ob.downcast::().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::().unwrap(); } assert_eq!(32 + 42 + 123, values_sum); diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 03e20644ee5..d427edbae5a 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -376,7 +376,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::()?); } Ok(v) @@ -656,7 +656,7 @@ mod tests { let ob = (&v).into_pyobject(py).unwrap(); let seq = ob.downcast::().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::().unwrap()); idx += 1; } @@ -688,7 +688,7 @@ mod tests { let concat_seq = seq.concat(seq).unwrap(); assert_eq!(6, concat_seq.len().unwrap()); let concat_v: Vec = 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::().unwrap()); } }); @@ -703,7 +703,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::().unwrap()); } }); @@ -718,7 +718,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::().unwrap()); } }); diff --git a/tests/test_proto_methods.rs b/tests/test_proto_methods.rs index 9fa6a8b888e..11214ec0dc6 100644 --- a/tests/test_proto_methods.rs +++ b/tests/test_proto_methods.rs @@ -853,7 +853,7 @@ impl DefaultedContains { PyList::new(py, ["a", "b", "c"]) .unwrap() .as_ref() - .iter() + .try_iter() .unwrap() .into() } @@ -868,7 +868,7 @@ impl NoContains { PyList::new(py, ["a", "b", "c"]) .unwrap() .as_ref() - .iter() + .try_iter() .unwrap() .into() }