deprecate {PyList, PyTuple}::get_item and implement ::get and ::get_unchecked

Author: Aviram Hassan

src/types/dict.rs

@@ -644,8 +644,8 @@ mod test {
         let mut value_sum = 0;
         for el in dict.items().iter() {
             let tuple = el.cast_as::<PyTuple>().unwrap();
-            key_sum += tuple.get_item(0).extract::<i32>().unwrap();
-            value_sum += tuple.get_item(1).extract::<i32>().unwrap();
+            key_sum += tuple.get(0).unwrap().extract::<i32>().unwrap();
+            value_sum += tuple.get(1).unwrap().extract::<i32>().unwrap();
         }
         assert_eq!(7 + 8 + 9, key_sum);
         assert_eq!(32 + 42 + 123, value_sum);

src/types/list.rs

@@ -69,6 +69,7 @@ impl PyList {
     /// Gets the item at the specified index.
     ///
     /// Panics if the index is out of range.
+    #[deprecated = "use PyList::get, which does not panic"]
     pub fn get_item(&self, index: isize) -> &PyAny {
         assert!(index >= 0 && index < self.len() as isize);
         unsafe {
@@ -83,6 +84,24 @@ impl PyList {
         }
     }
 
+    /// Gets the list item at the specified index.
+    pub fn get(&self, index: usize) -> PyResult<&PyAny> {
+        unsafe {
+            let item = ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t);
+            self.py().from_borrowed_ptr_or_err(item)
+        }
+    }
+
+    /// Gets the list item at the specified index. Undefined behavior on bad index. Use with caution.
+    /// # Safety
+    ///
+    /// Caller must verify that the index is within the bounds of the list.
+    #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+    pub unsafe fn get_unchecked(&self, index: usize) -> &PyAny {
+        let item = ffi::PyList_GET_ITEM(self.as_ptr(), index as Py_ssize_t);
+        self.py().from_borrowed_ptr(item)
+    }
+
     /// Sets the item at the specified index.
     ///
     /// Panics if the index is out of range.
@@ -142,16 +161,19 @@ impl PyList {
 /// Used by `PyList::iter()`.
 pub struct PyListIterator<'a> {
     list: &'a PyList,
-    index: isize,
+    index: usize,
 }
 
 impl<'a> Iterator for PyListIterator<'a> {
     type Item = &'a PyAny;
 
     #[inline]
     fn next(&mut self) -> Option<&'a PyAny> {
-        if self.index < self.list.len() as isize {
-            let item = self.list.get_item(self.index);
+        if self.index < self.list.len() {
+            #[cfg(any(Py_LIMITED_API, PyPy))]
+            let item = self.list.get(self.index).expect("tuple.get failed");
+            #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+            let item = unsafe { self.list.get_unchecked(self.index) };
             self.index += 1;
             Some(item)
         } else {
@@ -217,10 +239,10 @@ mod test {
         let gil = Python::acquire_gil();
         let py = gil.python();
         let list = PyList::new(py, &[2, 3, 5, 7]);
-        assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
-        assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
-        assert_eq!(7, list.get_item(3).extract::<i32>().unwrap());
+        assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
+        assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
+        assert_eq!(7, list.get(3).unwrap().extract::<i32>().unwrap());
     }
 
     #[test]
@@ -236,19 +258,10 @@ mod test {
         let gil = Python::acquire_gil();
         let py = gil.python();
         let list = PyList::new(py, &[2, 3, 5, 7]);
-        assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
-        assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
-        assert_eq!(7, list.get_item(3).extract::<i32>().unwrap());
-    }
-
-    #[test]
-    #[should_panic]
-    fn test_get_item_invalid() {
-        let gil = Python::acquire_gil();
-        let py = gil.python();
-        let list = PyList::new(py, &[2, 3, 5, 7]);
-        list.get_item(-1);
+        assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
+        assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
+        assert_eq!(7, list.get(3).unwrap().extract::<i32>().unwrap());
     }
 
     #[test]
@@ -257,9 +270,9 @@ mod test {
         let py = gil.python();
         let list = PyList::new(py, &[2, 3, 5, 7]);
         let val = 42i32.to_object(py);
-        assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
+        assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
         list.set_item(0, val).unwrap();
-        assert_eq!(42, list.get_item(0).extract::<i32>().unwrap());
+        assert_eq!(42, list.get(0).unwrap().extract::<i32>().unwrap());
     }
 
     #[test]
@@ -288,11 +301,11 @@ mod test {
         let list = PyList::new(py, &[2, 3, 5, 7]);
         let val = 42i32.to_object(py);
         assert_eq!(4, list.len());
-        assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
+        assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
         list.insert(0, val).unwrap();
         assert_eq!(5, list.len());
-        assert_eq!(42, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(2, list.get_item(1).extract::<i32>().unwrap());
+        assert_eq!(42, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(2, list.get(1).unwrap().extract::<i32>().unwrap());
     }
 
     #[test]
@@ -318,8 +331,8 @@ mod test {
         let py = gil.python();
         let list = PyList::new(py, &[2]);
         list.append(3).unwrap();
-        assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
+        assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
     }
 
     #[test]
@@ -397,15 +410,15 @@ mod test {
         let py = gil.python();
         let v = vec![7, 3, 2, 5];
         let list = PyList::new(py, &v);
-        assert_eq!(7, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
-        assert_eq!(2, list.get_item(2).extract::<i32>().unwrap());
-        assert_eq!(5, list.get_item(3).extract::<i32>().unwrap());
+        assert_eq!(7, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
+        assert_eq!(2, list.get(2).unwrap().extract::<i32>().unwrap());
+        assert_eq!(5, list.get(3).unwrap().extract::<i32>().unwrap());
         list.sort().unwrap();
-        assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
-        assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
-        assert_eq!(7, list.get_item(3).extract::<i32>().unwrap());
+        assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
+        assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
+        assert_eq!(7, list.get(3).unwrap().extract::<i32>().unwrap());
     }
 
     #[test]
@@ -414,15 +427,15 @@ mod test {
         let py = gil.python();
         let v = vec![2, 3, 5, 7];
         let list = PyList::new(py, &v);
-        assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
-        assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
-        assert_eq!(7, list.get_item(3).extract::<i32>().unwrap());
+        assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
+        assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
+        assert_eq!(7, list.get(3).unwrap().extract::<i32>().unwrap());
         list.reverse().unwrap();
-        assert_eq!(7, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(5, list.get_item(1).extract::<i32>().unwrap());
-        assert_eq!(3, list.get_item(2).extract::<i32>().unwrap());
-        assert_eq!(2, list.get_item(3).extract::<i32>().unwrap());
+        assert_eq!(7, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(5, list.get(1).unwrap().extract::<i32>().unwrap());
+        assert_eq!(3, list.get(2).unwrap().extract::<i32>().unwrap());
+        assert_eq!(2, list.get(3).unwrap().extract::<i32>().unwrap());
     }
 
     #[test]
@@ -431,7 +444,48 @@ mod test {
         let py = gil.python();
         let array: PyObject = [1, 2].into_py(py);
         let list = <PyList as PyTryFrom>::try_from(array.as_ref(py)).unwrap();
-        assert_eq!(1, list.get_item(0).extract::<i32>().unwrap());
-        assert_eq!(2, list.get_item(1).extract::<i32>().unwrap());
+        assert_eq!(1, list.get(0).unwrap().extract::<i32>().unwrap());
+        assert_eq!(2, list.get(1).unwrap().extract::<i32>().unwrap());
+    }
+
+    #[test]
+    fn test_list_get_invalid_index() {
+        pyo3::Python::with_gil(|py| {
+            let list = PyList::new(py, &[2, 3, 5, 7]);
+            let obj = list.get(5);
+            assert!(obj.is_err());
+            assert_eq!(
+                obj.unwrap_err().to_string(),
+                "IndexError: list index out of range"
+            );
+        });
+    }
+
+    #[test]
+    fn test_list_get_sanity() {
+        pyo3::Python::with_gil(|py| {
+            let list = PyList::new(py, &[2, 3, 5, 7]);
+            let obj = list.get(0);
+            assert_eq!(obj.unwrap().extract::<i32>().unwrap(), 2);
+        });
+    }
+
+    #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+    #[test]
+    fn test_list_get_unchecked_sanity() {
+        pyo3::Python::with_gil(|py| {
+            let list = PyList::new(py, &[2, 3, 5, 7]);
+            let obj = unsafe { list.get_unchecked(0) };
+            assert_eq!(obj.extract::<i32>().unwrap(), 2);
+        });
+    }
+
+    #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+    #[test]
+    fn test_list_get_unchecked_invalid_index() {
+        pyo3::Python::with_gil(|py| {
+            let list = PyList::new(py, &[2, 3, 5, 7]);
+            unsafe { list.get_unchecked(5) };
+        });
     }
 }

src/types/tuple.rs

@@ -76,6 +76,7 @@ impl PyTuple {
     /// Gets the tuple item at the specified index.
     ///
     /// Panics if the index is out of range.
+    #[deprecated = "use PyTuple::get, which does not panic"]
     pub fn get_item(&self, index: usize) -> &PyAny {
         assert!(index < self.len());
         unsafe {
@@ -88,6 +89,24 @@ impl PyTuple {
         }
     }
 
+    /// Gets the tuple item at the specified index.
+    pub fn get(&self, index: usize) -> PyResult<&PyAny> {
+        unsafe {
+            let item = ffi::PyTuple_GetItem(self.as_ptr(), index as Py_ssize_t);
+            self.py().from_borrowed_ptr_or_err(item)
+        }
+    }
+
+    /// Gets the tuple item at the specified index. Undefined behavior on bad index. Use with caution.
+    /// # Safety
+    ///
+    /// Caller must verify that the index is within the bounds of the list.
+    #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+    pub unsafe fn get_unchecked(&self, index: usize) -> &PyAny {
+        let item = ffi::PyTuple_GET_ITEM(self.as_ptr(), index as Py_ssize_t);
+        self.py().from_borrowed_ptr(item)
+    }
+
     /// Returns `self` as a slice of objects.
     #[cfg(not(Py_LIMITED_API))]
     #[cfg_attr(docsrs, doc(cfg(not(Py_LIMITED_API))))]
@@ -124,7 +143,10 @@ impl<'a> Iterator for PyTupleIterator<'a> {
     #[inline]
     fn next(&mut self) -> Option<&'a PyAny> {
         if self.index < self.length {
-            let item = self.tuple.get_item(self.index);
+            #[cfg(any(Py_LIMITED_API, PyPy))]
+            let item = self.tuple.get(self.index).expect("tuple.get failed");
+            #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+            let item = unsafe { self.tuple.get_unchecked(self.index) };
             self.index += 1;
             Some(item)
         } else {
@@ -200,9 +222,12 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
         {
             let t = <PyTuple as PyTryFrom>::try_from(obj)?;
             if t.len() == $length {
-                Ok((
-                    $(t.get_item($n).extract::<$T>()?,)+
-                ))
+                #[cfg(any(Py_LIMITED_API, PyPy))]
+                return Ok(($(t.get($n)?.extract::<$T>()?,)+));
+
+                #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+                unsafe {return Ok(($(t.get_unchecked($n).extract::<$T>()?,)+));}
+
             } else {
                 Err(wrong_tuple_length(t, $length))
             }
@@ -458,4 +483,39 @@ mod test {
             );
         })
     }
+
+    #[test]
+    fn test_tuple_get_invalid_index() {
+        pyo3::Python::with_gil(|py| {
+            let ob = (1, 2, 3).to_object(py);
+            let tuple = <PyTuple as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
+            let obj = tuple.get(5);
+            assert!(obj.is_err());
+            assert_eq!(
+                obj.unwrap_err().to_string(),
+                "IndexError: tuple index out of range"
+            );
+        });
+    }
+
+    #[test]
+    fn test_tuple_get_sanity() {
+        pyo3::Python::with_gil(|py| {
+            let ob = (1, 2, 3).to_object(py);
+            let tuple = <PyTuple as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
+            let obj = tuple.get(0);
+            assert_eq!(obj.unwrap().extract::<i32>().unwrap(), 1);
+        });
+    }
+
+    #[cfg(not(any(Py_LIMITED_API, PyPy)))]
+    #[test]
+    fn test_tuple_get_unchecked_sanity() {
+        pyo3::Python::with_gil(|py| {
+            let ob = (1, 2, 3).to_object(py);
+            let tuple = <PyTuple as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
+            let obj = unsafe { tuple.get_unchecked(0) };
+            assert_eq!(obj.extract::<i32>().unwrap(), 1);
+        });
+    }
 }