Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PyAnyMethods.try_iter and deprecate .iter #4553

Merged
merged 3 commits into from
Oct 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4553.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
33 changes: 32 additions & 1 deletion src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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())) }
}
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
Loading