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
PyO3 · Oct 4, 2024 · 75bd3f2 · 75bd3f2
1 parent d4ad8a2
commit 75bd3f2
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 19 deletions.
diff --git a/newsfragments/4553.fixed.md 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
@@ -77,7 +77,7 @@ fn bench_collect_generic_iterator(b: &mut Bencher<'_>) {
 
         b.iter(|| {
             collection
-                .iter()
+                .try_iter()
                 .unwrap()
                 .collect::<PyResult<Vec<_>>>()
                 .unwrap()

diff --git 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::<A::Item>()?);
     }
     Ok(sv)

diff --git 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<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.
@@ -1381,10 +1408,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())) }
     }

diff --git 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<Vec<usize>> = list
-///         .iter()?
+///         .try_iter()?
 ///         .map(|i| i.and_then(|i|i.extract::<usize>()))
 ///         .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::<usize>().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
@@ -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();
@@ -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);
@@ -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);

diff --git 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::<T>()?);
     }
     Ok(v)
@@ -656,7 +656,7 @@ mod tests {
             let ob = (&v).into_pyobject(py).unwrap();
             let seq = ob.downcast::<PySequence>().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;
             }
@@ -688,7 +688,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());
             }
         });
@@ -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::<char>().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::<String>().unwrap());
             }
         });

diff --git 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()
     }