deprecate IntoPy in favor or IntoPyObject (#4618)

* deprecate `IntoPy` in favor of `IntoPyObject` * add newsfragment * apply review comments Co-authored-by: David Hewitt <mail@davidhewitt.dev> --------- Co-authored-by: David Hewitt <mail@davidhewitt.dev>
PyO3 · Oct 29, 2024 · ca2f639 · ca2f639
1 parent 0aa13c8
commit ca2f639
Show file tree

Hide file tree

Showing 67 changed files with 533 additions and 414 deletions.
diff --git a/guide/src/class.md b/guide/src/class.md
@@ -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)
@@ -1284,11 +1284,13 @@ 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`.
+WARNING: `Py::new` and `.into_pyobject` 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_pyobject`.
 
 ```rust
 # use pyo3::prelude::*;
@@ -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)

diff --git a/guide/src/class/protocols.md b/guide/src/class/protocols.md
@@ -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()),
+                CompareOp::Ne => Ok((self.0 != other.0).into_pyobject(py)?.into_any()),
+                _ => Ok(PyNotImplemented::get(py).into_any()),
             }
         }
     }

diff --git a/guide/src/conversions/traits.md b/guide/src/conversions/traits.md
@@ -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

diff --git a/guide/src/migration.md b/guide/src/migration.md
@@ -1217,6 +1217,7 @@ Python::with_gil(|py| {
 After, some type annotations may be necessary:
 
 ```rust
+# #![allow(deprecated)]
 # use pyo3::prelude::*;
 #
 # fn main() {
@@ -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
@@ -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);

diff --git a/newsfragments/4618.changed.md b/newsfragments/4618.changed.md
@@ -0,0 +1 @@
+deprecate `IntoPy` in favor of `IntoPyObject`
diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
@@ -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 {
@@ -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();
@@ -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)
                 },
             }
         })
@@ -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)
                  },
             }
         })
@@ -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)

diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs
@@ -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)
         }
     };
 

diff --git a/src/conversion.rs b/src/conversion.rs
@@ -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
@@ -41,18 +42,21 @@ use std::convert::Infallible;
 /// # use pyo3::ffi;
 /// #
 /// Python::with_gil(|py| {
-///     let ptr: *mut ffi::PyObject = 0xabad1dea_u32.into_py(py).as_ptr();
+///     // ERROR: calling `.as_ptr()` will throw away the temporary object and leave `ptr` dangling.
+///     let ptr: *mut ffi::PyObject = 0xabad1dea_u32.into_pyobject(py)?.as_ptr();
 ///
 ///     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.
 ///
@@ -100,6 +104,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)]
@@ -121,6 +126,7 @@ pub trait ToPyObject {
 /// This is useful for types like enums that can carry different types.
 ///
 /// ```rust
+/// #![allow(deprecated)]
 /// use pyo3::prelude::*;
 ///
 /// enum Value {
@@ -161,6 +167,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;
@@ -503,6 +513,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()

diff --git a/src/conversions/chrono.rs b/src/conversions/chrono.rs
@@ -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,
@@ -72,6 +72,7 @@ impl ToPyObject for Duration {
     }
 }
 
+#[allow(deprecated)]
 impl IntoPy<PyObject> for Duration {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
@@ -178,6 +179,7 @@ impl ToPyObject for NaiveDate {
     }
 }
 
+#[allow(deprecated)]
 impl IntoPy<PyObject> for NaiveDate {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
@@ -244,6 +246,7 @@ impl ToPyObject for NaiveTime {
     }
 }
 
+#[allow(deprecated)]
 impl IntoPy<PyObject> for NaiveTime {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
@@ -320,6 +323,7 @@ impl ToPyObject for NaiveDateTime {
     }
 }
 
+#[allow(deprecated)]
 impl IntoPy<PyObject> for NaiveDateTime {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
@@ -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()
@@ -505,6 +510,7 @@ impl ToPyObject for FixedOffset {
     }
 }
 
+#[allow(deprecated)]
 impl IntoPy<PyObject> for FixedOffset {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
@@ -589,6 +595,7 @@ impl ToPyObject for Utc {
     }
 }
 
+#[allow(deprecated)]
 impl IntoPy<PyObject> for Utc {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
@@ -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);
                 })
             }
@@ -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);
                 })
             }
@@ -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();
@@ -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();