From 70a378538345bf3317d5c149a8fd0f88fc341ddb Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 11 Sep 2020 18:51:57 +0200 Subject: [PATCH 1/4] improve PyIterator docs a little --- src/types/iterator.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 0a5c3a80..4dac1f08 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -9,6 +9,10 @@ use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python}; /// Unlike other Python objects, this class includes a `Python<'p>` token /// so that `PyIterator` can implement the Rust `Iterator` trait. /// +/// This means that you can't use `PyIterator` in many places where other +/// types like `PyList` can automatically be extracted from objects, such +/// as function arguments. +/// /// # Example /// /// ```rust @@ -29,7 +33,9 @@ use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python}; pub struct PyIterator<'p>(&'p PyAny); impl<'p> PyIterator<'p> { - /// Constructs a `PyIterator` from a Python iterator object. + /// Constructs a `PyIterator` from a Python iterable object. + /// + /// Equivalent to Python's built-in `iter` function. pub fn from_object(py: Python<'p>, obj: &T) -> PyResult> where T: AsPyPointer, From 7b90a9b13ec0984dc3af889363430919b5890f20 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 11 Sep 2020 19:03:01 +0200 Subject: [PATCH 2/4] implement PyIterator without additional lifetime This lets us treat it no different from other types like PySequence. --- src/types/any.rs | 2 +- src/types/iterator.rs | 58 +++++++++++++++++++++++-------------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/types/any.rs b/src/types/any.rs index cd973b00..e5f4ae84 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -362,7 +362,7 @@ impl PyAny { /// /// This is typically a new iterator but if the argument is an iterator, /// this returns itself. - pub fn iter(&self) -> PyResult { + pub fn iter(&self) -> PyResult<&PyIterator> { PyIterator::from_object(self.py(), self) } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 4dac1f08..59506012 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -2,17 +2,12 @@ // // based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython -use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python}; +use crate::{ + ffi, AsPyPointer, PyAny, PyDowncastError, PyErr, PyNativeType, PyResult, PyTryFrom, Python, +}; /// A Python iterator object. /// -/// Unlike other Python objects, this class includes a `Python<'p>` token -/// so that `PyIterator` can implement the Rust `Iterator` trait. -/// -/// This means that you can't use `PyIterator` in many places where other -/// types like `PyList` can automatically be extracted from objects, such -/// as function arguments. -/// /// # Example /// /// ```rust @@ -30,31 +25,24 @@ use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python}; /// # } /// ``` #[derive(Debug)] -pub struct PyIterator<'p>(&'p PyAny); +#[repr(transparent)] +pub struct PyIterator(PyAny); +pyobject_native_type_named!(PyIterator); +pyobject_native_type_extract!(PyIterator); -impl<'p> PyIterator<'p> { +impl PyIterator { /// Constructs a `PyIterator` from a Python iterable object. /// /// Equivalent to Python's built-in `iter` function. - pub fn from_object(py: Python<'p>, obj: &T) -> PyResult> + pub fn from_object<'p, T>(py: Python<'p>, obj: &T) -> PyResult<&'p PyIterator> where T: AsPyPointer, { - let iter = unsafe { - // This looks suspicious, but is actually correct. Even though ptr is an owned - // reference, PyIterator takes ownership of the reference and decreases the count - // in its Drop implementation. - // - // Therefore we must use from_borrowed_ptr_or_err instead of from_owned_ptr_or_err so - // that the GILPool does not take ownership of the reference. - py.from_borrowed_ptr_or_err(ffi::PyObject_GetIter(obj.as_ptr()))? - }; - - Ok(PyIterator(iter)) + unsafe { py.from_owned_ptr_or_err(ffi::PyObject_GetIter(obj.as_ptr())) } } } -impl<'p> Iterator for PyIterator<'p> { +impl<'p> Iterator for &'p PyIterator { type Item = PyResult<&'p PyAny>; /// Retrieves the next item from an iterator. @@ -79,10 +67,26 @@ impl<'p> Iterator for PyIterator<'p> { } } -/// Dropping a `PyIterator` instance decrements the reference count on the object by 1. -impl<'p> Drop for PyIterator<'p> { - fn drop(&mut self) { - unsafe { ffi::Py_DECREF(self.0.as_ptr()) } +impl<'v> PyTryFrom<'v> for PyIterator { + fn try_from>(value: V) -> Result<&'v PyIterator, PyDowncastError<'v>> { + let value = value.into(); + unsafe { + if ffi::PyIter_Check(value.as_ptr()) != 0 { + Ok(::try_from_unchecked(value)) + } else { + Err(PyDowncastError::new(value, "Iterator")) + } + } + } + + fn try_from_exact>(value: V) -> Result<&'v PyIterator, PyDowncastError<'v>> { + ::try_from(value) + } + + #[inline] + unsafe fn try_from_unchecked>(value: V) -> &'v PyIterator { + let ptr = value.into() as *const _ as *const PyIterator; + &*ptr } } From 482ee3a8b202b824e3f8b85fa31e11cca94d42fa Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Mon, 12 Oct 2020 14:35:27 +0100 Subject: [PATCH 3/4] Add changelog entry and test --- CHANGELOG.md | 3 +++ src/types/iterator.rs | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60918324..7b4774cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed +- Change `PyIterator` to be consistent with other native types: it is now used as `&PyIterator` instead of `PyIterator<'a>`. [#1176](https://github.com/PyO3/pyo3/pull/1176) + ### Removed - Remove unused `python3` feature. [#1209](https://github.com/PyO3/pyo3/pull/1209) diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 59506012..4666d39e 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -96,8 +96,7 @@ mod tests { use crate::exceptions::PyTypeError; use crate::gil::GILPool; use crate::types::{PyDict, PyList}; - use crate::Python; - use crate::ToPyObject; + use crate::{Python, Py, PyAny, ToPyObject, PyTryFrom}; use indoc::indoc; #[test] @@ -199,4 +198,13 @@ mod tests { assert!(err.is_instance::(py)) } + + #[test] + fn iterator_try_from() { + let gil_guard = Python::acquire_gil(); + let py = gil_guard.python(); + let obj: Py = vec![10, 20].to_object(py).as_ref(py).iter().unwrap().into(); + let iter: &PyIterator = PyIterator::try_from(obj.as_ref(py)).unwrap(); + assert_eq!(obj, iter.into()); + } } From 1c84539ef9a84a5d2fdb66c6849c00322363fa17 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Mon, 12 Oct 2020 16:22:11 +0100 Subject: [PATCH 4/4] rustfmt --- src/types/iterator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 4666d39e..cc798ebf 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -96,7 +96,7 @@ mod tests { use crate::exceptions::PyTypeError; use crate::gil::GILPool; use crate::types::{PyDict, PyList}; - use crate::{Python, Py, PyAny, ToPyObject, PyTryFrom}; + use crate::{Py, PyAny, PyTryFrom, Python, ToPyObject}; use indoc::indoc; #[test]