Skip to content
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

deprecate IntoPy in favor or IntoPyObject #4618

Merged
merged 3 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 6 additions & 3 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -1258,8 +1258,8 @@ enum Shape {

# #[cfg(Py_3_10)]
Python::with_gil(|py| {
let circle = Shape::Circle { radius: 10.0 }.into_py(py);
let square = Shape::RegularPolygon(4, 10.0).into_py(py);
let circle = Shape::Circle { radius: 10.0 }.into_pyobject(py)?;
let square = Shape::RegularPolygon(4, 10.0).into_pyobject(py)?;
let cls = py.get_type::<Shape>();
pyo3::py_run!(py, circle square cls, r#"
assert isinstance(circle, cls)
Expand All @@ -1284,8 +1284,10 @@ Python::with_gil(|py| {

assert count_vertices(cls, circle) == 0
assert count_vertices(cls, square) == 4
"#)
"#);
# Ok::<_, PyErr>(())
})
# .unwrap();
```

WARNING: `Py::new` and `.into_py` are currently inconsistent. Note how the constructed value is _not_ an instance of the specific variant. For this reason, constructing values is only recommended using `.into_py`.
Icxolu marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -1404,6 +1406,7 @@ impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a
}
}

#[allow(deprecated)]
impl pyo3::IntoPy<PyObject> for MyClass {
fn into_py(self, py: pyo3::Python<'_>) -> pyo3::PyObject {
pyo3::IntoPy::into_py(pyo3::Py::new(py, self).unwrap(), py)
Expand Down
10 changes: 6 additions & 4 deletions guide/src/class/protocols.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,21 @@ given signatures should be interpreted as follows:

```rust
use pyo3::class::basic::CompareOp;
use pyo3::types::PyNotImplemented;

# use pyo3::prelude::*;
# use pyo3::BoundObject;
#
# #[pyclass]
# struct Number(i32);
#
#[pymethods]
impl Number {
fn __richcmp__(&self, other: &Self, op: CompareOp, py: Python<'_>) -> PyObject {
fn __richcmp__<'py>(&self, other: &Self, op: CompareOp, py: Python<'py>) -> PyResult<Borrowed<'py, 'py, PyAny>> {
match op {
CompareOp::Eq => (self.0 == other.0).into_py(py),
CompareOp::Ne => (self.0 != other.0).into_py(py),
_ => py.NotImplemented(),
CompareOp::Eq => Ok((self.0 == other.0).into_pyobject(py)?.into_any()),
Copy link
Member

Choose a reason for hiding this comment

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

Does this work to get to PyObject? It might be a touch simpler for users to understand.

Suggested change
CompareOp::Eq => Ok((self.0 == other.0).into_pyobject(py)?.into_any()),
CompareOp::Eq => Ok((self.0 == other.0).into_pyobject(py)?.into()),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, currently it does not.

Copy link
Member

Choose a reason for hiding this comment

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

I think interestingly #4593 might help here. But again an improvement for after 0.23

CompareOp::Ne => Ok((self.0 != other.0).into_pyobject(py)?.into_any()),
_ => Ok(PyNotImplemented::get(py).into_any()),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions guide/src/conversions/traits.md
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ use pyo3::prelude::*;
# #[allow(dead_code)]
struct MyPyObjectWrapper(PyObject);

#[allow(deprecated)]
impl IntoPy<PyObject> for MyPyObjectWrapper {
fn into_py(self, py: Python<'_>) -> PyObject {
self.0
Expand Down
3 changes: 3 additions & 0 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,7 @@ Python::with_gil(|py| {
After, some type annotations may be necessary:

```rust
# #![allow(deprecated)]
# use pyo3::prelude::*;
#
# fn main() {
Expand Down Expand Up @@ -1695,6 +1696,7 @@ After
# #[allow(dead_code)]
struct MyPyObjectWrapper(PyObject);

# #[allow(deprecated)]
impl IntoPy<PyObject> for MyPyObjectWrapper {
fn into_py(self, _py: Python<'_>) -> PyObject {
self.0
Expand All @@ -1714,6 +1716,7 @@ let obj = PyObject::from_py(1.234, py);

After:
```rust
# #![allow(deprecated)]
# use pyo3::prelude::*;
# Python::with_gil(|py| {
let obj: PyObject = 1.234.into_py(py);
Expand Down
1 change: 1 addition & 0 deletions newsfragments/4618.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
deprecate `IntoPy` in favor of `IntoPyObject`
44 changes: 31 additions & 13 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,7 @@ fn impl_complex_enum(
.collect();

quote! {
#[allow(deprecated)]
impl #pyo3_path::IntoPy<#pyo3_path::PyObject> for #cls {
fn into_py(self, py: #pyo3_path::Python) -> #pyo3_path::PyObject {
match self {
Expand Down Expand Up @@ -1349,13 +1350,11 @@ fn impl_complex_enum_tuple_variant_getitem(
let match_arms: Vec<_> = (0..num_fields)
.map(|i| {
let field_access = format_ident!("_{}", i);
quote! {
#i => ::std::result::Result::Ok(
#pyo3_path::IntoPy::into_py(
#variant_cls::#field_access(slf)?
, py)
)

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)
}
})
.collect();
Expand Down Expand Up @@ -1837,10 +1836,16 @@ fn pyclass_richcmp_arms(
.map(|span| {
quote_spanned! { span =>
#pyo3_path::pyclass::CompareOp::Eq => {
::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val == other, py))
#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::pyclass::CompareOp::Ne => {
::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val != other, py))
#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)
},
}
})
Expand All @@ -1855,16 +1860,28 @@ fn pyclass_richcmp_arms(
.map(|ord| {
quote_spanned! { ord.span() =>
#pyo3_path::pyclass::CompareOp::Gt => {
::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val > other, py))
#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::pyclass::CompareOp::Lt => {
::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val < other, py))
#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::pyclass::CompareOp::Le => {
::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val <= other, py))
#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::pyclass::CompareOp::Ge => {
::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val >= other, py))
#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)
},
}
})
Expand Down Expand Up @@ -2145,6 +2162,7 @@ impl<'a> PyClassImplsBuilder<'a> {
// If #cls is not extended type, we allow Self->PyObject conversion
if attr.options.extends.is_none() {
quote! {
#[allow(deprecated)]
impl #pyo3_path::IntoPy<#pyo3_path::PyObject> for #cls {
fn into_py(self, py: #pyo3_path::Python<'_>) -> #pyo3_path::PyObject {
#pyo3_path::IntoPy::into_py(#pyo3_path::Py::new(py, self).unwrap(), py)
Expand Down
5 changes: 4 additions & 1 deletion pyo3-macros-backend/src/pyimpl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,10 @@ 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> {
::std::result::Result::Ok(#pyo3_path::IntoPy::into_py(#cls::#member, py))
#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)
}
};

Expand Down
24 changes: 17 additions & 7 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ use std::convert::Infallible;
///
/// ```rust
/// use pyo3::prelude::*;
/// use pyo3::types::PyString;
/// use pyo3::ffi;
///
/// Python::with_gil(|py| {
/// let s: Py<PyString> = "foo".into_py(py);
/// let s = "foo".into_pyobject(py)?;
/// let ptr = s.as_ptr();
///
/// let is_really_a_pystring = unsafe { ffi::PyUnicode_CheckExact(ptr) };
/// assert_eq!(is_really_a_pystring, 1);
/// });
/// # Ok::<_, PyErr>(())
/// })
/// # .unwrap();
/// ```
///
/// # Safety
Expand All @@ -41,18 +42,20 @@ use std::convert::Infallible;
/// # use pyo3::ffi;
/// #
/// Python::with_gil(|py| {
/// let ptr: *mut ffi::PyObject = 0xabad1dea_u32.into_py(py).as_ptr();
/// let ptr: *mut ffi::PyObject = 0xabad1dea_u32.into_pyobject(py)?.as_ptr();
Icxolu marked this conversation as resolved.
Show resolved Hide resolved
///
/// let isnt_a_pystring = unsafe {
/// // `ptr` is dangling, this is UB
/// ffi::PyUnicode_CheckExact(ptr)
/// };
/// # assert_eq!(isnt_a_pystring, 0);
/// });
/// # assert_eq!(isnt_a_pystring, 0);
/// # Ok::<_, PyErr>(())
/// })
/// # .unwrap();
/// ```
///
/// This happens because the pointer returned by `as_ptr` does not carry any lifetime information
/// and the Python object is dropped immediately after the `0xabad1dea_u32.into_py(py).as_ptr()`
/// and the Python object is dropped immediately after the `0xabad1dea_u32.into_pyobject(py).as_ptr()`
/// expression is evaluated. To fix the problem, bind Python object to a local variable like earlier
/// to keep the Python object alive until the end of its scope.
///
Expand Down Expand Up @@ -100,6 +103,7 @@ pub trait ToPyObject {
/// However, it may not be desirable to expose the existence of `Number` to Python code.
/// `IntoPy` allows us to define a conversion to an appropriate Python object.
/// ```rust
/// #![allow(deprecated)]
/// use pyo3::prelude::*;
///
/// # #[allow(dead_code)]
Expand All @@ -121,6 +125,7 @@ pub trait ToPyObject {
/// This is useful for types like enums that can carry different types.
///
/// ```rust
/// #![allow(deprecated)]
/// use pyo3::prelude::*;
///
/// enum Value {
Expand Down Expand Up @@ -161,6 +166,10 @@ pub trait ToPyObject {
note = "if you do not own `{Self}` you can perform a manual conversion to one of the types in `pyo3::types::*`"
)
)]
#[deprecated(
since = "0.23.0",
note = "`IntoPy` is going to be replaced by `IntoPyObject`. See the migration guide (https://pyo3.rs/v0.23/migration) for more information."
)]
pub trait IntoPy<T>: Sized {
/// Performs the conversion.
fn into_py(self, py: Python<'_>) -> T;
Expand Down Expand Up @@ -503,6 +512,7 @@ where
}

/// Converts `()` to an empty Python tuple.
#[allow(deprecated)]
impl IntoPy<Py<PyTuple>> for () {
fn into_py(self, py: Python<'_>) -> Py<PyTuple> {
PyTuple::empty(py).unbind()
Expand Down
29 changes: 18 additions & 11 deletions src/conversions/chrono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ use crate::types::{
timezone_utc, PyDate, PyDateAccess, PyDateTime, PyDelta, PyDeltaAccess, PyTime, PyTimeAccess,
PyTzInfo, PyTzInfoAccess,
};
#[allow(deprecated)]
use crate::ToPyObject;
use crate::{ffi, Bound, FromPyObject, IntoPy, PyAny, PyErr, PyObject, PyResult, Python};
use crate::{ffi, Bound, FromPyObject, PyAny, PyErr, PyObject, PyResult, Python};
#[cfg(Py_LIMITED_API)]
use crate::{intern, DowncastError};
#[allow(deprecated)]
use crate::{IntoPy, ToPyObject};
use chrono::offset::{FixedOffset, Utc};
use chrono::{
DateTime, Datelike, Duration, NaiveDate, NaiveDateTime, NaiveTime, Offset, TimeZone, Timelike,
Expand All @@ -72,6 +72,7 @@ impl ToPyObject for Duration {
}
}

#[allow(deprecated)]
impl IntoPy<PyObject> for Duration {
#[inline]
fn into_py(self, py: Python<'_>) -> PyObject {
Expand Down Expand Up @@ -178,6 +179,7 @@ impl ToPyObject for NaiveDate {
}
}

#[allow(deprecated)]
impl IntoPy<PyObject> for NaiveDate {
#[inline]
fn into_py(self, py: Python<'_>) -> PyObject {
Expand Down Expand Up @@ -244,6 +246,7 @@ impl ToPyObject for NaiveTime {
}
}

#[allow(deprecated)]
impl IntoPy<PyObject> for NaiveTime {
#[inline]
fn into_py(self, py: Python<'_>) -> PyObject {
Expand Down Expand Up @@ -320,6 +323,7 @@ impl ToPyObject for NaiveDateTime {
}
}

#[allow(deprecated)]
impl IntoPy<PyObject> for NaiveDateTime {
#[inline]
fn into_py(self, py: Python<'_>) -> PyObject {
Expand Down Expand Up @@ -411,6 +415,7 @@ impl<Tz: TimeZone> ToPyObject for DateTime<Tz> {
}
}

#[allow(deprecated)]
impl<Tz: TimeZone> IntoPy<PyObject> for DateTime<Tz> {
fn into_py(self, py: Python<'_>) -> PyObject {
self.into_pyobject(py).unwrap().into_any().unbind()
Expand Down Expand Up @@ -505,6 +510,7 @@ impl ToPyObject for FixedOffset {
}
}

#[allow(deprecated)]
impl IntoPy<PyObject> for FixedOffset {
#[inline]
fn into_py(self, py: Python<'_>) -> PyObject {
Expand Down Expand Up @@ -589,6 +595,7 @@ impl ToPyObject for Utc {
}
}

#[allow(deprecated)]
impl IntoPy<PyObject> for Utc {
#[inline]
fn into_py(self, py: Python<'_>) -> PyObject {
Expand Down Expand Up @@ -1408,8 +1415,8 @@ mod tests {
// python values of durations (from -999999999 to 999999999 days),
Python::with_gil(|py| {
let dur = Duration::days(days);
let py_delta = dur.into_py(py);
let roundtripped: Duration = py_delta.extract(py).expect("Round trip");
let py_delta = dur.into_pyobject(py).unwrap();
let roundtripped: Duration = py_delta.extract().expect("Round trip");
assert_eq!(dur, roundtripped);
})
}
Expand All @@ -1418,8 +1425,8 @@ mod tests {
fn test_fixed_offset_roundtrip(secs in -86399i32..=86399i32) {
Python::with_gil(|py| {
let offset = FixedOffset::east_opt(secs).unwrap();
let py_offset = offset.into_py(py);
let roundtripped: FixedOffset = py_offset.extract(py).expect("Round trip");
let py_offset = offset.into_pyobject(py).unwrap();
let roundtripped: FixedOffset = py_offset.extract().expect("Round trip");
assert_eq!(offset, roundtripped);
})
}
Expand Down Expand Up @@ -1504,8 +1511,8 @@ mod tests {
if let (Some(date), Some(time)) = (date_opt, time_opt) {
let dt: DateTime<Utc> = NaiveDateTime::new(date, time).and_utc();
// Wrap in CatchWarnings to avoid into_py firing warning for truncated leap second
let py_dt = CatchWarnings::enter(py, |_| Ok(dt.into_py(py))).unwrap();
let roundtripped: DateTime<Utc> = py_dt.extract(py).expect("Round trip");
let py_dt = CatchWarnings::enter(py, |_| dt.into_pyobject(py)).unwrap();
let roundtripped: DateTime<Utc> = py_dt.extract().expect("Round trip");
// Leap seconds are not roundtripped
let expected_roundtrip_time = micro.checked_sub(1_000_000).map(|micro| NaiveTime::from_hms_micro_opt(hour, min, sec, micro).unwrap()).unwrap_or(time);
let expected_roundtrip_dt: DateTime<Utc> = NaiveDateTime::new(date, expected_roundtrip_time).and_utc();
Expand All @@ -1532,8 +1539,8 @@ mod tests {
if let (Some(date), Some(time)) = (date_opt, time_opt) {
let dt: DateTime<FixedOffset> = NaiveDateTime::new(date, time).and_local_timezone(offset).unwrap();
// Wrap in CatchWarnings to avoid into_py firing warning for truncated leap second
let py_dt = CatchWarnings::enter(py, |_| Ok(dt.into_py(py))).unwrap();
let roundtripped: DateTime<FixedOffset> = py_dt.extract(py).expect("Round trip");
let py_dt = CatchWarnings::enter(py, |_| dt.into_pyobject(py)).unwrap();
let roundtripped: DateTime<FixedOffset> = py_dt.extract().expect("Round trip");
// Leap seconds are not roundtripped
let expected_roundtrip_time = micro.checked_sub(1_000_000).map(|micro| NaiveTime::from_hms_micro_opt(hour, min, sec, micro).unwrap()).unwrap_or(time);
let expected_roundtrip_dt: DateTime<FixedOffset> = NaiveDateTime::new(date, expected_roundtrip_time).and_local_timezone(offset).unwrap();
Expand Down
Loading
Loading