Skip to content

add IntoPyObjectExt trait #4708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 7 additions & 28 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1367,10 +1367,7 @@ fn impl_complex_enum_tuple_variant_getitem(
.map(|i| {
let field_access = format_ident!("_{}", i);
quote! { #i =>
#pyo3_path::IntoPyObject::into_pyobject(#variant_cls::#field_access(slf)?, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(#variant_cls::#field_access(slf)?, py)
}
})
.collect();
Expand Down Expand Up @@ -1852,16 +1849,10 @@ fn pyclass_richcmp_arms(
.map(|span| {
quote_spanned! { span =>
#pyo3_path::pyclass::CompareOp::Eq => {
#pyo3_path::IntoPyObject::into_pyobject(self_val == other, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(self_val == other, py)
},
#pyo3_path::pyclass::CompareOp::Ne => {
#pyo3_path::IntoPyObject::into_pyobject(self_val != other, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(self_val != other, py)
},
}
})
Expand All @@ -1876,28 +1867,16 @@ fn pyclass_richcmp_arms(
.map(|ord| {
quote_spanned! { ord.span() =>
#pyo3_path::pyclass::CompareOp::Gt => {
#pyo3_path::IntoPyObject::into_pyobject(self_val > other, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(self_val > other, py)
},
#pyo3_path::pyclass::CompareOp::Lt => {
#pyo3_path::IntoPyObject::into_pyobject(self_val < other, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(self_val < other, py)
},
#pyo3_path::pyclass::CompareOp::Le => {
#pyo3_path::IntoPyObject::into_pyobject(self_val <= other, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(self_val <= other, py)
},
#pyo3_path::pyclass::CompareOp::Ge => {
#pyo3_path::IntoPyObject::into_pyobject(self_val >= other, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(self_val >= other, py)
},
}
})
Expand Down
5 changes: 1 addition & 4 deletions pyo3-macros-backend/src/pyimpl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,7 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec, ctx: &Ctx) -> MethodAndMe

let associated_method = quote! {
fn #wrapper_ident(py: #pyo3_path::Python<'_>) -> #pyo3_path::PyResult<#pyo3_path::PyObject> {
#pyo3_path::IntoPyObject::into_pyobject(#cls::#member, py)
.map(#pyo3_path::BoundObject::into_any)
.map(#pyo3_path::BoundObject::unbind)
.map_err(::std::convert::Into::into)
#pyo3_path::IntoPyObjectExt::into_py_any(#cls::#member, py)
}
};

Expand Down
77 changes: 65 additions & 12 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,23 @@ pub trait IntoPy<T>: Sized {

/// Defines a conversion from a Rust type to a Python object, which may fail.
///
/// This trait has `#[derive(IntoPyObject)]` to automatically implement it for simple types and
/// `#[derive(IntoPyObjectRef)]` to implement the same for references.
///
/// It functions similarly to std's [`TryInto`] trait, but requires a [GIL token](Python)
/// as an argument.
///
/// The [`into_pyobject`][IntoPyObject::into_pyobject] method is designed for maximum flexibility and efficiency; it
/// - allows for a concrete Python type to be returned (the [`Target`][IntoPyObject::Target] associated type)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to finish this list, had to run from my pc just now

/// - allows for the smart pointer containing the Python object to be either `Bound<'py, Self::Target>` or `Borrowed<'a, 'py, Self::Target>`
/// to avoid unnecessary reference counting overhead
/// - allows for a custom error type to be returned in the event of a conversion error to avoid
/// unnecessarily creating a Python exception
///
/// # See also
///
/// - The [`IntoPyObjectExt`] trait, which provides convenience methods for common usages of
/// `IntoPyObject` which erase type information and convert errors to `PyErr`.
#[cfg_attr(
diagnostic_namespace,
diagnostic::on_unimplemented(
Expand Down Expand Up @@ -227,12 +242,7 @@ pub trait IntoPyObject<'py>: Sized {
I: IntoIterator<Item = Self> + AsRef<[Self]>,
I::IntoIter: ExactSizeIterator<Item = Self>,
{
let mut iter = iter.into_iter().map(|e| {
e.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::into_bound)
.map_err(Into::into)
});
let mut iter = iter.into_iter().map(|e| e.into_bound_py_any(py));
let list = crate::types::list::try_new_from_iter(py, &mut iter);
list.map(Bound::into_any)
}
Expand All @@ -250,12 +260,7 @@ pub trait IntoPyObject<'py>: Sized {
I: IntoIterator<Item = Self> + AsRef<[<Self as private::Reference>::BaseType]>,
I::IntoIter: ExactSizeIterator<Item = Self>,
{
let mut iter = iter.into_iter().map(|e| {
e.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::into_bound)
.map_err(Into::into)
});
let mut iter = iter.into_iter().map(|e| e.into_bound_py_any(py));
let list = crate::types::list::try_new_from_iter(py, &mut iter);
list.map(Bound::into_any)
}
Expand Down Expand Up @@ -347,6 +352,54 @@ where
}
}

mod into_pyobject_ext {
pub trait Sealed {}
impl<'py, T> Sealed for T where T: super::IntoPyObject<'py> {}
}

/// Convenience methods for common usages of [`IntoPyObject`]. Every type that implements
/// [`IntoPyObject`] also implements this trait.
///
/// These methods:
/// - Drop type information from the output, returning a `PyAny` object.
/// - Always convert the `Error` type to `PyErr`, which may incur a performance penalty but it
/// more convenient in contexts where the `?` operator would produce a `PyErr` anyway.
pub trait IntoPyObjectExt<'py>: IntoPyObject<'py> + into_pyobject_ext::Sealed {
/// Converts `self` into an owned Python object, dropping type information.
#[inline]
fn into_bound_py_any(self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> {
match self.into_pyobject(py) {
Ok(obj) => Ok(obj.into_any().into_bound()),
Err(err) => Err(err.into()),
}
}

/// Converts `self` into an owned Python object, dropping type information and unbinding it
/// from the `'py` lifetime.
#[inline]
fn into_py_any(self, py: Python<'py>) -> PyResult<Py<PyAny>> {
match self.into_pyobject(py) {
Ok(obj) => Ok(obj.into_any().unbind()),
Err(err) => Err(err.into()),
}
}

/// Converts `self` into a Python object.
///
/// This is equivalent to calling [`into_pyobject`][IntoPyObject::into_pyobject] followed
/// with `.map_err(Into::into)` to convert the error type to [`PyErr`]. This is helpful
/// for generic code which wants to make use of the `?` operator.
#[inline]
fn into_pyobject_or_pyerr(self, py: Python<'py>) -> PyResult<Self::Output> {
match self.into_pyobject(py) {
Ok(obj) => Ok(obj),
Err(err) => Err(err.into()),
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think into_unbound_py_any would also probably be useful (there's a customer in Coroutine::new it looks like).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems reasonable to me; it certainly is common to .unbind() in pydantic-core (so that data can be stored in structs, mostly).

}

impl<'py, T> IntoPyObjectExt<'py> for T where T: IntoPyObject<'py> {}

/// Extract a type from a Python object.
///
///
Expand Down
28 changes: 6 additions & 22 deletions src/conversions/either.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@
#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
use crate::{
conversion::IntoPyObject, exceptions::PyTypeError, types::any::PyAnyMethods, Bound,
BoundObject, FromPyObject, PyAny, PyErr, PyObject, PyResult, Python,
exceptions::PyTypeError, types::any::PyAnyMethods, Bound, BoundObject, FromPyObject,
IntoPyObject, IntoPyObjectExt, PyAny, PyErr, PyObject, PyResult, Python,
};
#[allow(deprecated)]
use crate::{IntoPy, ToPyObject};
Expand Down Expand Up @@ -82,16 +82,8 @@ where

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
match self {
Either::Left(l) => l
.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::into_bound)
.map_err(Into::into),
Either::Right(r) => r
.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::into_bound)
.map_err(Into::into),
Either::Left(l) => l.into_bound_py_any(py),
Either::Right(r) => r.into_bound_py_any(py),
}
}
}
Expand All @@ -108,16 +100,8 @@ where

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
match self {
Either::Left(l) => l
.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::into_bound)
.map_err(Into::into),
Either::Right(r) => r
.into_pyobject(py)
.map(BoundObject::into_any)
.map(BoundObject::into_bound)
.map_err(Into::into),
Either::Left(l) => l.into_bound_py_any(py),
Either::Right(r) => r.into_bound_py_any(py),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@
#![doc = concat!("[Features chapter of the guide]: https://pyo3.rs/v", env!("CARGO_PKG_VERSION"), "/features.html#features-reference \"Features Reference - PyO3 user guide\"")]
//! [`Ungil`]: crate::marker::Ungil
pub use crate::class::*;
pub use crate::conversion::{AsPyPointer, FromPyObject, IntoPyObject};
pub use crate::conversion::{AsPyPointer, FromPyObject, IntoPyObject, IntoPyObjectExt};
#[allow(deprecated)]
pub use crate::conversion::{IntoPy, ToPyObject};
pub use crate::err::{DowncastError, DowncastIntoError, PyErr, PyErrArguments, PyResult, ToPyErr};
Expand Down
50 changes: 9 additions & 41 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::type_object::{PyTypeCheck, PyTypeInfo};
#[cfg(not(any(PyPy, GraalPy)))]
use crate::types::PySuper;
use crate::types::{PyDict, PyIterator, PyList, PyString, PyTuple, PyType};
use crate::{err, ffi, Borrowed, BoundObject, Python};
use crate::{err, ffi, Borrowed, BoundObject, IntoPyObjectExt, Python};
use std::cell::UnsafeCell;
use std::cmp::Ordering;
use std::os::raw::c_int;
Expand Down Expand Up @@ -1000,11 +1000,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
.into_pyobject(py)
.map_err(Into::into)?
.as_borrowed(),
value
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
value.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
)
}

Expand Down Expand Up @@ -1057,11 +1053,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
let py = self.py();
inner(
self,
other
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
other.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
)
}

Expand All @@ -1083,11 +1075,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
let py = self.py();
inner(
self,
other
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
other.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
compare_op,
)
}
Expand Down Expand Up @@ -1198,11 +1186,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
let py = self.py();
inner(
self,
other
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
other.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
)
}

Expand All @@ -1227,16 +1211,8 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
let py = self.py();
inner(
self,
other
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
modulus
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
other.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
modulus.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
)
}

Expand Down Expand Up @@ -1383,11 +1359,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
.map_err(Into::into)?
.into_any()
.as_borrowed(),
value
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
value.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
)
}

Expand Down Expand Up @@ -1574,11 +1546,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
let py = self.py();
inner(
self,
value
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
.as_borrowed(),
value.into_pyobject_or_pyerr(py)?.into_any().as_borrowed(),
)
}

Expand Down
Loading