Skip to content

Commit

Permalink
migrate PySet to IntoPyObject (#4536)
Browse files Browse the repository at this point in the history 
* migrate `PySetMethods` trait bounds

* migrate constructor trait bounds
  • Loading branch information
@Icxolu
Icxolu authored Sep 30, 2024
1 parent 8a6855e commit 116170a
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 145 deletions.
24 changes: 4 additions & 20 deletions src/conversions/hashbrown.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,7 @@ where
type Error = PyErr;

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
try_new_from_iter(
py,
self.into_iter().map(|item| {
item.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::unbind)
.map_err(Into::into)
}),
)
try_new_from_iter(py, self)
}
}

Expand All @@ -169,15 +161,7 @@ where
type Error = PyErr;

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
try_new_from_iter(
py,
self.into_iter().map(|item| {
item.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::unbind)
.map_err(Into::into)
}),
)
try_new_from_iter(py, self)
}
}

Expand Down Expand Up @@ -272,11 +256,11 @@ mod tests {
#[test]
fn test_extract_hashbrown_hashset() {
Python::with_gil(|py| {
let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap();
let set = PySet::new(py, [1, 2, 3, 4, 5]).unwrap();
let hash_set: hashbrown::HashSet<usize> = set.extract().unwrap();
assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());

let set = PyFrozenSet::new(py, &[1, 2, 3, 4, 5]).unwrap();
let set = PyFrozenSet::new(py, [1, 2, 3, 4, 5]).unwrap();
let hash_set: hashbrown::HashSet<usize> = set.extract().unwrap();
assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
});
Expand Down
50 changes: 9 additions & 41 deletions src/conversions/std/set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
set::{new_from_iter, try_new_from_iter, PySetMethods},
PyFrozenSet, PySet,
},
BoundObject, FromPyObject, IntoPy, PyAny, PyErr, PyObject, PyResult, Python, ToPyObject,
FromPyObject, IntoPy, PyAny, PyErr, PyObject, PyResult, Python, ToPyObject,
};

impl<T, S> ToPyObject for collections::HashSet<T, S>
Expand Down Expand Up @@ -64,15 +64,7 @@ where
type Error = PyErr;

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
try_new_from_iter(
py,
self.into_iter().map(|item| {
item.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::unbind)
.map_err(Into::into)
}),
)
try_new_from_iter(py, self)
}
}

Expand All @@ -86,15 +78,7 @@ where
type Error = PyErr;

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
try_new_from_iter(
py,
self.iter().map(|item| {
item.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::unbind)
.map_err(Into::into)
}),
)
try_new_from_iter(py, self.iter())
}
}

Expand Down Expand Up @@ -147,15 +131,7 @@ where
type Error = PyErr;

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
try_new_from_iter(
py,
self.into_iter().map(|item| {
item.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::unbind)
.map_err(Into::into)
}),
)
try_new_from_iter(py, self)
}
}

Expand All @@ -168,15 +144,7 @@ where
type Error = PyErr;

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
try_new_from_iter(
py,
self.iter().map(|item| {
item.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::unbind)
.map_err(Into::into)
}),
)
try_new_from_iter(py, self.iter())
}
}

Expand Down Expand Up @@ -212,11 +180,11 @@ mod tests {
#[test]
fn test_extract_hashset() {
Python::with_gil(|py| {
let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap();
let set = PySet::new(py, [1, 2, 3, 4, 5]).unwrap();
let hash_set: HashSet<usize> = set.extract().unwrap();
assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());

let set = PyFrozenSet::new(py, &[1, 2, 3, 4, 5]).unwrap();
let set = PyFrozenSet::new(py, [1, 2, 3, 4, 5]).unwrap();
let hash_set: HashSet<usize> = set.extract().unwrap();
assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
});
Expand All @@ -225,11 +193,11 @@ mod tests {
#[test]
fn test_extract_btreeset() {
Python::with_gil(|py| {
let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap();
let set = PySet::new(py, [1, 2, 3, 4, 5]).unwrap();
let hash_set: BTreeSet<usize> = set.extract().unwrap();
assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());

let set = PyFrozenSet::new(py, &[1, 2, 3, 4, 5]).unwrap();
let set = PyFrozenSet::new(py, [1, 2, 3, 4, 5]).unwrap();
let hash_set: BTreeSet<usize> = set.extract().unwrap();
assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
});
Expand Down
10 changes: 10 additions & 0 deletions src/instance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ pub trait BoundObject<'py, T>: bound_object_sealed::Sealed {
fn into_any(self) -> Self::Any;
/// Turn this smart pointer into a strong reference pointer
fn into_ptr(self) -> *mut ffi::PyObject;
/// Turn this smart pointer into a borrowed reference pointer
fn as_ptr(&self) -> *mut ffi::PyObject;
/// Turn this smart pointer into an owned [`Py<T>`]
fn unbind(self) -> Py<T>;
}
Expand Down Expand Up @@ -616,6 +618,10 @@ impl<'py, T> BoundObject<'py, T> for Bound<'py, T> {
self.into_ptr()
}

fn as_ptr(&self) -> *mut ffi::PyObject {
self.as_ptr()
}

fn unbind(self) -> Py<T> {
self.unbind()
}
Expand Down Expand Up @@ -835,6 +841,10 @@ impl<'a, 'py, T> BoundObject<'py, T> for Borrowed<'a, 'py, T> {
(*self).to_owned().into_ptr()
}

fn as_ptr(&self) -> *mut ffi::PyObject {
(*self).as_ptr()
}

fn unbind(self) -> Py<T> {
(*self).to_owned().unbind()
}
Expand Down
98 changes: 57 additions & 41 deletions src/types/frozenset.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use crate::conversion::IntoPyObject;
use crate::types::PyIterator;
use crate::{
err::{self, PyErr, PyResult},
ffi,
ffi_ptr_ext::FfiPtrExt,
py_result_ext::PyResultExt,
types::any::PyAnyMethods,
Bound, PyAny, PyObject, Python, ToPyObject,
Bound, PyAny, Python, ToPyObject,
};
use crate::{Borrowed, BoundObject};
use std::ptr;

/// Allows building a Python `frozenset` one item at a time
Expand All @@ -27,15 +29,21 @@ impl<'py> PyFrozenSetBuilder<'py> {
/// Adds an element to the set.
pub fn add<K>(&mut self, key: K) -> PyResult<()>
where
K: ToPyObject,
K: IntoPyObject<'py>,
{
fn inner(frozenset: &Bound<'_, PyFrozenSet>, key: PyObject) -> PyResult<()> {
fn inner(frozenset: &Bound<'_, PyFrozenSet>, key: Borrowed<'_, '_, PyAny>) -> PyResult<()> {
err::error_on_minusone(frozenset.py(), unsafe {
ffi::PySet_Add(frozenset.as_ptr(), key.as_ptr())
})
}

inner(&self.py_frozen_set, key.to_object(self.py_frozen_set.py()))
inner(
&self.py_frozen_set,
key.into_pyobject(self.py_frozen_set.py())
.map_err(Into::into)?
.into_any()
.as_borrowed(),
)
}

/// Finish building the set and take ownership of its current value
Expand Down Expand Up @@ -83,11 +91,14 @@ impl PyFrozenSet {
///
/// May panic when running out of memory.
#[inline]
pub fn new<'a, 'p, T: ToPyObject + 'a>(
py: Python<'p>,
elements: impl IntoIterator<Item = &'a T>,
) -> PyResult<Bound<'p, PyFrozenSet>> {
new_from_iter(py, elements)
pub fn new<'py, T>(
py: Python<'py>,
elements: impl IntoIterator<Item = T>,
) -> PyResult<Bound<'py, PyFrozenSet>>
where
T: IntoPyObject<'py>,
{
try_new_from_iter(py, elements)
}

/// Deprecated name for [`PyFrozenSet::new`].
Expand All @@ -97,7 +108,7 @@ impl PyFrozenSet {
py: Python<'p>,
elements: impl IntoIterator<Item = &'a T>,
) -> PyResult<Bound<'p, PyFrozenSet>> {
Self::new(py, elements)
Self::new(py, elements.into_iter().map(|e| e.to_object(py)))
}

/// Creates a new empty frozen set
Expand Down Expand Up @@ -139,7 +150,7 @@ pub trait PyFrozenSetMethods<'py>: crate::sealed::Sealed {
/// This is equivalent to the Python expression `key in self`.
fn contains<K>(&self, key: K) -> PyResult<bool>
where
K: ToPyObject;
K: IntoPyObject<'py>;

/// Returns an iterator of values in this set.
fn iter(&self) -> BoundFrozenSetIterator<'py>;
Expand All @@ -153,9 +164,12 @@ impl<'py> PyFrozenSetMethods<'py> for Bound<'py, PyFrozenSet> {

fn contains<K>(&self, key: K) -> PyResult<bool>
where
K: ToPyObject,
K: IntoPyObject<'py>,
{
fn inner(frozenset: &Bound<'_, PyFrozenSet>, key: Bound<'_, PyAny>) -> PyResult<bool> {
fn inner(
frozenset: &Bound<'_, PyFrozenSet>,
key: Borrowed<'_, '_, PyAny>,
) -> PyResult<bool> {
match unsafe { ffi::PySet_Contains(frozenset.as_ptr(), key.as_ptr()) } {
1 => Ok(true),
0 => Ok(false),
Expand All @@ -164,7 +178,13 @@ impl<'py> PyFrozenSetMethods<'py> for Bound<'py, PyFrozenSet> {
}

let py = self.py();
inner(self, key.to_object(py).into_bound(py))
inner(
self,
key.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
)
}

fn iter(&self) -> BoundFrozenSetIterator<'py> {
Expand Down Expand Up @@ -229,31 +249,27 @@ impl<'py> ExactSizeIterator for BoundFrozenSetIterator<'py> {
}

#[inline]
pub(crate) fn new_from_iter<T: ToPyObject>(
py: Python<'_>,
pub(crate) fn try_new_from_iter<'py, T>(
py: Python<'py>,
elements: impl IntoIterator<Item = T>,
) -> PyResult<Bound<'_, PyFrozenSet>> {
fn inner<'py>(
py: Python<'py>,
elements: &mut dyn Iterator<Item = PyObject>,
) -> PyResult<Bound<'py, PyFrozenSet>> {
let set = unsafe {
// We create the `Py` pointer because its Drop cleans up the set if user code panics.
ffi::PyFrozenSet_New(std::ptr::null_mut())
.assume_owned_or_err(py)?
.downcast_into_unchecked()
};
let ptr = set.as_ptr();

for obj in elements {
err::error_on_minusone(py, unsafe { ffi::PySet_Add(ptr, obj.as_ptr()) })?;
}

Ok(set)
) -> PyResult<Bound<'py, PyFrozenSet>>
where
T: IntoPyObject<'py>,
{
let set = unsafe {
// We create the `Py` pointer because its Drop cleans up the set if user code panics.
ffi::PyFrozenSet_New(std::ptr::null_mut())
.assume_owned_or_err(py)?
.downcast_into_unchecked()
};
let ptr = set.as_ptr();

for e in elements {
let obj = e.into_pyobject(py).map_err(Into::into)?;
err::error_on_minusone(py, unsafe { ffi::PySet_Add(ptr, obj.as_ptr()) })?;
}

let mut iter = elements.into_iter().map(|e| e.to_object(py));
inner(py, &mut iter)
Ok(set)
}

#[cfg(test)]
Expand All @@ -263,7 +279,7 @@ mod tests {
#[test]
fn test_frozenset_new_and_len() {
Python::with_gil(|py| {
let set = PyFrozenSet::new(py, &[1]).unwrap();
let set = PyFrozenSet::new(py, [1]).unwrap();
assert_eq!(1, set.len());

let v = vec![1];
Expand All @@ -283,15 +299,15 @@ mod tests {
#[test]
fn test_frozenset_contains() {
Python::with_gil(|py| {
let set = PyFrozenSet::new(py, &[1]).unwrap();
let set = PyFrozenSet::new(py, [1]).unwrap();
assert!(set.contains(1).unwrap());
});
}

#[test]
fn test_frozenset_iter() {
Python::with_gil(|py| {
let set = PyFrozenSet::new(py, &[1]).unwrap();
let set = PyFrozenSet::new(py, [1]).unwrap();

for el in set {
assert_eq!(1i32, el.extract::<i32>().unwrap());
Expand All @@ -302,7 +318,7 @@ mod tests {
#[test]
fn test_frozenset_iter_bound() {
Python::with_gil(|py| {
let set = PyFrozenSet::new(py, &[1]).unwrap();
let set = PyFrozenSet::new(py, [1]).unwrap();

for el in &set {
assert_eq!(1i32, el.extract::<i32>().unwrap());
Expand All @@ -313,7 +329,7 @@ mod tests {
#[test]
fn test_frozenset_iter_size_hint() {
Python::with_gil(|py| {
let set = PyFrozenSet::new(py, &[1]).unwrap();
let set = PyFrozenSet::new(py, [1]).unwrap();
let mut iter = set.iter();

// Exact size
Expand Down
Loading

0 comments on commit 116170a

Please sign in to comment.