luxolus
/
pyo3
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1803 from PyO3/pysequence_usize 

PySequence: use usize everywhere, fix in-place methods
This commit is contained in:
David Hewitt 2021-08-24 07:09:58 +01:00 committed by GitHub
parent bbc23e3bf4 ead86bd666
commit cfd6a5bc35
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 140 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
CHANGELOG.md
View File

@ -16,8 +16,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Changed
- Change `PyErr::fetch()` to return `Option<PyErr>`. [#1717](https://github.com/PyO3/pyo3/pull/1717)
- `PyList`, `PyTuple` and `PySequence`'s `get_item` now accepts only `usize` indices instead of `isize`. [#1733](https://github.com/PyO3/pyo3/pull/1733)
- `PyList` and `PyTuple`'s `get_item` now return `PyResult<&PyAny> instead of panicking. [#1733](https://github.com/PyO3/pyo3/pull/1733)
- `PyList`, `PyTuple` and `PySequence`'s APIs now accepts only `usize` indices instead of `isize`.
[#1733](https://github.com/PyO3/pyo3/pull/1733), [#1802](https://github.com/PyO3/pyo3/pull/1802),
[#1803](https://github.com/PyO3/pyo3/pull/1803)
- `PyList` and `PyTuple`'s `get_item` now return `PyResult<&PyAny>` instead of panicking. [#1733](https://github.com/PyO3/pyo3/pull/1733)
- `PySequence`'s `in_place_repeat` and `in_place_concat` now return the result instead of `()`, which is needed
in case of immutable sequences. [#1803](https://github.com/PyO3/pyo3/pull/1803)
- Deprecate `PyTuple::split_from`. [#1804](https://github.com/PyO3/pyo3/pull/1804)
## [0.14.3] - 2021-08-22

188
src/types/sequence.rs
View File

@ -1,7 +1,8 @@
// Copyright (c) 2017-present PyO3 Project and Contributors
use crate::err::{self, PyDowncastError, PyErr, PyResult};
use crate::ffi::{self, Py_ssize_t};
use crate::ffi;
use crate::internal_tricks::get_ssize_index;
use crate::types::{PyAny, PyList, PyTuple};
use crate::AsPyPointer;
use crate::{FromPyObject, PyTryFrom, ToBorrowedObject};
@ -17,13 +18,12 @@ impl PySequence {
///
/// This is equivalent to the Python expression `len(self)`.
#[inline]
#[allow(clippy::len_without_is_empty)]
pub fn len(&self) -> PyResult<isize> {
pub fn len(&self) -> PyResult<usize> {
let v = unsafe { ffi::PySequence_Size(self.as_ptr()) };
if v == -1 {
Err(PyErr::api_call_failed(self.py()))
} else {
Ok(v as isize)
Ok(v as usize)
}
}
@ -52,48 +52,56 @@ impl PySequence {
/// Returns the result of repeating a sequence object `count` times.
///
/// This is equivalent to the Python expression `self * count`.
/// NB: Python accepts negative counts; it returns an empty Sequence.
#[inline]
pub fn repeat(&self, count: isize) -> PyResult<&PySequence> {
pub fn repeat(&self, count: usize) -> PyResult<&PySequence> {
unsafe {
let ptr = self
.py()
.from_owned_ptr_or_err::<PyAny>(ffi::PySequence_Repeat(
self.as_ptr(),
count as Py_ssize_t,
get_ssize_index(count),
))?;
Ok(&*(ptr as *const PyAny as *const PySequence))
}
}
/// Concatenates `self` and `other` in place.
/// Concatenates `self` and `other`, in place if possible.
///
/// This is equivalent to the Python statement `self += other`.
/// This is equivalent to the Python expression `self.__iadd__(other)`.
///
/// The Python statement `self += other` is syntactic sugar for `self =
/// self.__iadd__(other)`. `__iadd__` should modify and return `self` if
/// possible, but create and return a new object if not.
#[inline]
pub fn in_place_concat(&self, other: &PySequence) -> PyResult<()> {
pub fn in_place_concat(&self, other: &PySequence) -> PyResult<&PySequence> {
unsafe {
let ptr = ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr());
if ptr.is_null() {
Err(PyErr::api_call_failed(self.py()))
} else {
Ok(())
}
let ptr = self
.py()
.from_owned_ptr_or_err::<PyAny>(ffi::PySequence_InPlaceConcat(
self.as_ptr(),
other.as_ptr(),
))?;
Ok(&*(ptr as *const PyAny as *const PySequence))
}
}
/// Repeats the sequence object `count` times and updates `self`.
/// Repeats the sequence object `count` times and updates `self`, if possible.
///
/// This is equivalent to the Python statement `self *= count`.
/// NB: Python accepts negative counts; it empties the Sequence.
/// This is equivalent to the Python expression `self.__imul__(other)`.
///
/// The Python statement `self *= other` is syntactic sugar for `self =
/// self.__imul__(other)`. `__imul__` should modify and return `self` if
/// possible, but create and return a new object if not.
#[inline]
pub fn in_place_repeat(&self, count: isize) -> PyResult<()> {
pub fn in_place_repeat(&self, count: usize) -> PyResult<&PySequence> {
unsafe {
let ptr = ffi::PySequence_InPlaceRepeat(self.as_ptr(), count as Py_ssize_t);
if ptr.is_null() {
Err(PyErr::api_call_failed(self.py()))
} else {
Ok(())
}
let ptr = self
.py()
.from_owned_ptr_or_err::<PyAny>(ffi::PySequence_InPlaceRepeat(
self.as_ptr(),
get_ssize_index(count),
))?;
Ok(&*(ptr as *const PyAny as *const PySequence))
}
}
@ -103,8 +111,10 @@ impl PySequence {
#[inline]
pub fn get_item(&self, index: usize) -> PyResult<&PyAny> {
unsafe {
self.py()
.from_owned_ptr_or_err(ffi::PySequence_GetItem(self.as_ptr(), index as Py_ssize_t))
self.py().from_owned_ptr_or_err(ffi::PySequence_GetItem(
self.as_ptr(),
get_ssize_index(index),
))
}
}
@ -112,12 +122,12 @@ impl PySequence {
///
/// This is equivalent to the Python expression `self[begin:end]`.
#[inline]
pub fn get_slice(&self, begin: isize, end: isize) -> PyResult<&PyAny> {
pub fn get_slice(&self, begin: usize, end: usize) -> PyResult<&PyAny> {
unsafe {
self.py().from_owned_ptr_or_err(ffi::PySequence_GetSlice(
self.as_ptr(),
begin as Py_ssize_t,
end as Py_ssize_t,
get_ssize_index(begin),
get_ssize_index(end),
))
}
}
@ -126,7 +136,7 @@ impl PySequence {
///
/// This is equivalent to the Python statement `self[i] = v`.
#[inline]
pub fn set_item<I>(&self, i: isize, item: I) -> PyResult<()>
pub fn set_item<I>(&self, i: usize, item: I) -> PyResult<()>
where
I: ToBorrowedObject,
{
@ -134,7 +144,7 @@ impl PySequence {
item.with_borrowed_ptr(self.py(), |item| {
err::error_on_minusone(
self.py(),
ffi::PySequence_SetItem(self.as_ptr(), i as Py_ssize_t, item),
ffi::PySequence_SetItem(self.as_ptr(), get_ssize_index(i), item),
)
})
}
@ -144,11 +154,11 @@ impl PySequence {
///
/// This is equivalent to the Python statement `del self[i]`.
#[inline]
pub fn del_item(&self, i: isize) -> PyResult<()> {
pub fn del_item(&self, i: usize) -> PyResult<()> {
unsafe {
err::error_on_minusone(
self.py(),
ffi::PySequence_DelItem(self.as_ptr(), i as Py_ssize_t),
ffi::PySequence_DelItem(self.as_ptr(), get_ssize_index(i)),
)
}
}
@ -157,14 +167,14 @@ impl PySequence {
///
/// This is equivalent to the Python statement `self[i1:i2] = v`.
#[inline]
pub fn set_slice(&self, i1: isize, i2: isize, v: &PyAny) -> PyResult<()> {
pub fn set_slice(&self, i1: usize, i2: usize, v: &PyAny) -> PyResult<()> {
unsafe {
err::error_on_minusone(
self.py(),
ffi::PySequence_SetSlice(
self.as_ptr(),
i1 as Py_ssize_t,
i2 as Py_ssize_t,
get_ssize_index(i1),
get_ssize_index(i2),
v.as_ptr(),
),
)
@ -175,11 +185,11 @@ impl PySequence {
///
/// This is equivalent to the Python statement `del self[i1:i2]`.
#[inline]
pub fn del_slice(&self, i1: isize, i2: isize) -> PyResult<()> {
pub fn del_slice(&self, i1: usize, i2: usize) -> PyResult<()> {
unsafe {
err::error_on_minusone(
self.py(),
ffi::PySequence_DelSlice(self.as_ptr(), i1 as Py_ssize_t, i2 as Py_ssize_t),
ffi::PySequence_DelSlice(self.as_ptr(), get_ssize_index(i1), get_ssize_index(i2)),
)
}
}
@ -330,14 +340,14 @@ impl<'v> PyTryFrom<'v> for PySequence {
#[cfg(test)]
mod tests {
use crate::types::PySequence;
use crate::types::{PyList, PySequence};
use crate::AsPyPointer;
use crate::Python;
use crate::{PyObject, PyTryFrom, ToPyObject};
fn get_object() -> PyObject {
// Convenience function for getting a single unique object
Python::with_gil(|py| -> PyObject {
Python::with_gil(|py| {
let obj = py.eval("object()", None, None).unwrap();
obj.to_object(py)
@ -372,6 +382,19 @@ mod tests {
});
}
#[test]
fn test_seq_is_empty() {
Python::with_gil(|py| {
let list = vec![1].to_object(py);
let seq = list.cast_as::<PySequence>(py).unwrap();
assert!(!seq.is_empty().unwrap());
let vec: Vec<u32> = Vec::new();
let empty_list = vec.to_object(py);
let empty_seq = empty_list.cast_as::<PySequence>(py).unwrap();
assert!(empty_seq.is_empty().unwrap());
});
}
#[test]
fn test_seq_contains() {
Python::with_gil(|py| {
@ -407,10 +430,6 @@ mod tests {
});
}
// fn test_get_slice() {}
// fn test_set_slice() {}
// fn test_del_slice() {}
#[test]
fn test_seq_del_item() {
Python::with_gil(|py| {
@ -464,6 +483,54 @@ mod tests {
});
}
#[test]
fn test_seq_get_slice() {
Python::with_gil(|py| {
let v: Vec<i32> = vec![1, 1, 2, 3, 5, 8];
let ob = v.to_object(py);
let seq = ob.cast_as::<PySequence>(py).unwrap();
assert_eq!(
[1, 2, 3],
seq.get_slice(1, 4).unwrap().extract::<[i32; 3]>().unwrap()
);
assert_eq!(
[3, 5, 8],
seq.get_slice(3, 100)
.unwrap()
.extract::<[i32; 3]>()
.unwrap()
);
});
}
#[test]
fn test_set_slice() {
Python::with_gil(|py| {
let v: Vec<i32> = vec![1, 1, 2, 3, 5, 8];
let w: Vec<i32> = vec![7, 4];
let ob = v.to_object(py);
let seq = ob.cast_as::<PySequence>(py).unwrap();
let ins = w.to_object(py);
seq.set_slice(1, 4, ins.as_ref(py)).unwrap();
assert_eq!([1, 7, 4, 5, 8], seq.extract::<[i32; 5]>().unwrap());
seq.set_slice(3, 100, PyList::empty(py)).unwrap();
assert_eq!([1, 7, 4], seq.extract::<[i32; 3]>().unwrap());
});
}
#[test]
fn test_del_slice() {
Python::with_gil(|py| {
let v: Vec<i32> = vec![1, 1, 2, 3, 5, 8];
let ob = v.to_object(py);
let seq = ob.cast_as::<PySequence>(py).unwrap();
seq.del_slice(1, 4).unwrap();
assert_eq!([1, 5, 8], seq.extract::<[i32; 3]>().unwrap());
seq.del_slice(1, 100).unwrap();
assert_eq!([1], seq.extract::<[i32; 1]>().unwrap());
});
}
#[test]
fn test_seq_index() {
Python::with_gil(|py| {
@ -570,6 +637,22 @@ mod tests {
});
}
#[test]
fn test_seq_inplace() {
Python::with_gil(|py| {
let v = vec!["foo", "bar"];
let ob = v.to_object(py);
let seq = ob.cast_as::<PySequence>(py).unwrap();
let rep_seq = seq.in_place_repeat(3).unwrap();
assert_eq!(6, seq.len().unwrap());
assert_eq!(seq, rep_seq);
let conc_seq = seq.in_place_concat(seq).unwrap();
assert_eq!(12, seq.len().unwrap());
assert_eq!(seq, conc_seq);
});
}
#[test]
fn test_list_coercion() {
Python::with_gil(|py| {
@ -653,17 +736,4 @@ mod tests {
assert!(seq_from.list().is_ok());
});
}
#[test]
fn test_is_empty() {
Python::with_gil(|py| {
let list = vec![1].to_object(py);
let seq = list.cast_as::<PySequence>(py).unwrap();
assert!(!seq.is_empty().unwrap());
let vec: Vec<u32> = Vec::new();
let empty_list = vec.to_object(py);
let empty_seq = empty_list.cast_as::<PySequence>(py).unwrap();
assert!(empty_seq.is_empty().unwrap());
});
}
}

10
tests/test_class_attributes.rs
View File

@ -40,7 +40,7 @@ impl Foo {
}
#[classattr]
fn foo() -> Foo {
fn a_foo() -> Foo {
Foo { x: 1 }
}
}
@ -54,7 +54,7 @@ fn class_attributes() {
py_assert!(py, foo_obj, "foo_obj.RENAMED_CONST == 'foobar_2'");
py_assert!(py, foo_obj, "foo_obj.a == 5");
py_assert!(py, foo_obj, "foo_obj.B == 'bar'");
py_assert!(py, foo_obj, "foo_obj.foo.x == 1");
py_assert!(py, foo_obj, "foo_obj.a_foo.x == 1");
}
// Ignored because heap types are not immutable:
@ -71,7 +71,7 @@ fn class_attributes_are_immutable() {
#[pymethods]
impl Bar {
#[classattr]
fn foo() -> Foo {
fn a_foo() -> Foo {
Foo { x: 3 }
}
}
@ -83,7 +83,7 @@ fn recursive_class_attributes() {
let foo_obj = py.get_type::<Foo>();
let bar_obj = py.get_type::<Bar>();
py_assert!(py, foo_obj, "foo_obj.foo.x == 1");
py_assert!(py, foo_obj, "foo_obj.a_foo.x == 1");
py_assert!(py, foo_obj, "foo_obj.bar.x == 2");
py_assert!(py, bar_obj, "bar_obj.foo.x == 3");
py_assert!(py, bar_obj, "bar_obj.a_foo.x == 3");
}