Merge pull request #717 from kngwyu/pyset-iter

Implement iter for PySet and PyFrozenSet

src/types/dict.rs

@@ -150,23 +150,20 @@ impl PyDict {
         }
     }
 
-    /// Returns a iterator of (key, value) pairs in this dictionary
+    /// Returns a iterator of (key, value) pairs in this dictionary.
-    /// Note that it's unsafe to use when the dictionary might be changed
+    ///
-    /// by other python code.
+    /// Note that it's unsafe to use when the dictionary might be changed by other code.
     pub fn iter(&self) -> PyDictIterator {
-        let py = self.py();
         PyDictIterator {
-            dict: self.to_object(py),
+            dict: self.as_ref(),
             pos: 0,
-            py,
         }
     }
 }
 
 pub struct PyDictIterator<'py> {
-    dict: PyObject,
+    dict: &'py PyAny,
     pos: isize,
-    py: Python<'py>,
 }
 
 impl<'py> Iterator for PyDictIterator<'py> {
@@ -178,7 +175,7 @@ impl<'py> Iterator for PyDictIterator<'py> {
             let mut key: *mut ffi::PyObject = std::ptr::null_mut();
             let mut value: *mut ffi::PyObject = std::ptr::null_mut();
             if ffi::PyDict_Next(self.dict.as_ptr(), &mut self.pos, &mut key, &mut value) != 0 {
-                let py = self.py;
+                let py = self.dict.py();
                 Some((py.from_borrowed_ptr(key), py.from_borrowed_ptr(value)))
             } else {
                 None

src/types/list.rs

@@ -113,7 +113,7 @@ impl PyList {
         })
     }
 
-    /// Returns an iterator over the tuple items.
+    /// Returns an iterator over this list items.
     pub fn iter(&self) -> PyListIterator {
         PyListIterator {
             list: self,

src/types/set.rs

@@ -95,6 +95,17 @@ impl PySet {
     pub fn pop(&self) -> Option<PyObject> {
         unsafe { PyObject::from_owned_ptr_or_opt(self.py(), ffi::PySet_Pop(self.as_ptr())) }
     }
+
+    /// Returns an iterator of values in this set.
+    ///
+    /// Note that it can be unsafe to use when the set might be changed by other code.
+    #[cfg(not(Py_LIMITED_API))]
+    pub fn iter(&self) -> PySetIterator {
+        PySetIterator {
+            set: self.as_ref(),
+            pos: 0,
+        }
+    }
 }
 
 #[cfg(not(Py_LIMITED_API))]
@@ -127,10 +138,7 @@ impl<'a> std::iter::IntoIterator for &'a PySet {
     type IntoIter = PySetIterator<'a>;
 
     fn into_iter(self) -> Self::IntoIter {
-        PySetIterator {
+        self.iter()
-            set: self.as_ref(),
-            pos: 0,
-        }
     }
 }
 
@@ -204,7 +212,16 @@ impl PyFrozenSet {
             }
         })
     }
+
+    /// Returns an iterator of values in this frozen set.
+    ///
+    /// Note that it can be unsafe to use when the set might be changed by other code.
+    #[cfg(not(Py_LIMITED_API))]
+    pub fn iter(&self) -> PySetIterator {
+        self.into_iter()
+    }
 }
+
 #[cfg(not(Py_LIMITED_API))]
 impl<'a> std::iter::IntoIterator for &'a PyFrozenSet {
     type Item = &'a PyAny;
@@ -222,9 +239,7 @@ impl<'a> std::iter::IntoIterator for &'a PyFrozenSet {
 mod test {
     use super::{PyFrozenSet, PySet};
     use crate::instance::AsPyRef;
-    use crate::objectprotocol::ObjectProtocol;
+    use crate::{ObjectProtocol, PyTryFrom, Python, ToPyObject};
-    use crate::Python;
-    use crate::{PyTryFrom, ToPyObject};
     use std::collections::HashSet;
 
     #[test]
@@ -317,9 +332,10 @@ mod test {
         let py = gil.python();
 
         let set = PySet::new(py, &[1]).unwrap();
-        // objectprotocol iteration
+
-        for el in set.iter().unwrap() {
+        // iter method
-            assert_eq!(1i32, el.unwrap().extract::<i32>().unwrap());
+        for el in set.iter() {
+            assert_eq!(1i32, el.extract().unwrap());
         }
 
         // intoiterator iteration
@@ -363,9 +379,9 @@ mod test {
 
         let set = PyFrozenSet::new(py, &[1]).unwrap();
 
-        // objectprotocol iteration
+        // iter method
-        for el in set.iter().unwrap() {
+        for el in set.iter() {
-            assert_eq!(1i32, el.unwrap().extract::<i32>().unwrap());
+            assert_eq!(1i32, el.extract::<i32>().unwrap());
         }
 
         // intoiterator iteration