Skip to content

Commit

Permalink
deprecate {PyList, PyTuple}::get_item and implement ::get and ::get_u…
Browse files Browse the repository at this point in the history 
…nchecked
  • Loading branch information
Aviram Hassan committed Jul 22, 2021
1 parent bd0e0d8 commit c4216a1
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 49 deletions.
4 changes: 2 additions & 2 deletions src/types/dict.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
139 changes: 94 additions & 45 deletions src/types/list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ impl PyList {
/// Gets the item at the specified index.
///
/// Panics if the index is out of range.
#[deprecated = "this is a deprecated alias for get and get_unchecked"]
pub fn get_item(&self, index: isize) -> &PyAny {
assert!(index >= 0 && index < self.len() as isize);
unsafe {
Expand All @@ -83,6 +84,23 @@ 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.
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
pub fn get_unchecked(&self, index: usize) -> &PyAny {
unsafe {
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.
Expand Down Expand Up @@ -142,16 +160,16 @@ 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() {
let item = self.list.get(self.index).expect("list.get failed");
self.index += 1;
Some(item)
} else {
Expand Down Expand Up @@ -217,10 +235,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]
Expand All @@ -236,19 +254,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]
Expand All @@ -257,9 +266,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]
Expand Down Expand Up @@ -288,11 +297,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]
Expand All @@ -318,8 +327,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]
Expand Down Expand Up @@ -397,15 +406,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]
Expand All @@ -414,15 +423,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]
Expand All @@ -431,7 +440,47 @@ 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() {
let gil = Python::acquire_gil();
let py = gil.python();
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() {
let gil = Python::acquire_gil();
let py = gil.python();
let list = PyList::new(py, &[2, 3, 5, 7]);
let obj = list.get(0);
assert_eq!(obj.unwrap().extract::<i32>().unwrap(), 2);
}

#[test]
fn test_list_get_unchecked_sanity() {
let gil = Python::acquire_gil();
let py = gil.python();
let list = PyList::new(py, &[2, 3, 5, 7]);
let obj = 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() {
let gil = Python::acquire_gil();
let py = gil.python();
let list = PyList::new(py, &[2, 3, 5, 7]);
list.get_unchecked(5);
}
}
57 changes: 55 additions & 2 deletions src/types/tuple.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl PyTuple {
/// Gets the tuple item at the specified index.
///
/// Panics if the index is out of range.
#[deprecated = "this is a deprecated alias for get and get_unchecked"]
pub fn get_item(&self, index: usize) -> &PyAny {
assert!(index < self.len());
unsafe {
Expand All @@ -88,6 +89,23 @@ 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.
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
pub fn get_unchecked(&self, index: usize) -> &PyAny {
unsafe {
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))))]
Expand Down Expand Up @@ -124,7 +142,7 @@ 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);
let item = self.tuple.get(self.index).expect("tuple.get failed");
self.index += 1;
Some(item)
} else {
Expand Down Expand Up @@ -201,7 +219,7 @@ 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>()?,)+
$(t.get($n)?.extract::<$T>()?,)+
))
} else {
Err(wrong_tuple_length(t, $length))
Expand Down Expand Up @@ -458,4 +476,39 @@ mod test {
);
})
}

#[test]
fn test_tuple_get_invalid_index() {
let gil = Python::acquire_gil();
let py = gil.python();
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() {
let gil = Python::acquire_gil();
let py = gil.python();
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() {
let gil = Python::acquire_gil();
let py = gil.python();
let ob = (1, 2, 3).to_object(py);
let tuple = <PyTuple as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
let obj = tuple.get_unchecked(0);
assert_eq!(obj.extract::<i32>().unwrap(), 1);
}
}

0 comments on commit c4216a1

Please sign in to comment.