initial migration of the trait bounds to IntoPyObject (`PyAnyMethod…

…s`) (#4480) * initial migration of the trait bounds to `IntoPyObject` * improve `PyBytes` comment wording Co-authored-by: Lily Foote <[email protected]> --------- Co-authored-by: Lily Foote <[email protected]>
PyO3 · Aug 24, 2024 · 7d399ff · 7d399ff
1 parent 1c98858
commit 7d399ff
Show file tree

Hide file tree

Showing 9 changed files with 425 additions and 164 deletions.
diff --git a/guide/src/migration.md b/guide/src/migration.md
@@ -133,6 +133,71 @@ This is purely additional and should just extend the possible return types.
 
 </details>
 
+### Python API trait bounds changed
+<details open>
+<summary><small>Click to expand</small></summary>
+
+PyO3 0.23 introduces a new unified `IntoPyObject` trait to convert Rust types into Python objects.
+Notable features of this new trait:
+- conversions can now return an error 
+- compared to `IntoPy<T>` the generic `T` moved into an associated type, so
+  - there is now only one way to convert a given type
+  - the output type is stronger typed and may return any Python type instead of just `PyAny`
+- byte collections are special handled and convert into `PyBytes` now, see [above](#macro-conversion-changed-for-byte-collections-vecu8-u8-n-and-smallvecu8-n)
+- `()` (unit) is now only special handled in return position and otherwise converts into an empty `PyTuple`
+
+All PyO3 provided types as well as `#[pyclass]`es already implement `IntoPyObject`. Other types will
+need to adapt an implementation of `IntoPyObject` to stay compatible with the Python APIs.
+
+
+Before: 
+```rust
+# use pyo3::prelude::*;
+# #[allow(dead_code)]
+struct MyPyObjectWrapper(PyObject);
+
+impl IntoPy<PyObject> for MyPyObjectWrapper {
+    fn into_py(self, py: Python<'_>) -> PyObject {
+        self.0
+    }
+}
+
+impl ToPyObject for MyPyObjectWrapper {
+    fn to_object(&self, py: Python<'_>) -> PyObject {
+        self.0.clone_ref(py)
+    }
+}
+```
+
+After:
+```rust
+# use pyo3::prelude::*;
+# #[allow(dead_code)]
+# struct MyPyObjectWrapper(PyObject);
+
+impl<'py> IntoPyObject<'py> for MyPyObjectWrapper {
+    type Target = PyAny; // the Python type
+    type Output = Bound<'py, Self::Target>; // in most cases this will be `Bound`
+    type Error = std::convert::Infallible;
+
+    fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
+        Ok(self.0.into_bound(py))
+    }
+}
+
+// `ToPyObject` implementations should be converted to implementations on reference types
+impl<'a, 'py> IntoPyObject<'py> for &'a MyPyObjectWrapper {
+    type Target = PyAny;
+    type Output = Borrowed<'a, 'py, Self::Target>; // `Borrowed` can be used to optimized reference counting
+    type Error = std::convert::Infallible;
+
+    fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
+        Ok(self.0.bind_borrowed(py))
+    }
+}
+```
+</details>
+
 ## from 0.21.* to 0.22
 
 ### Deprecation of `gil-refs` feature continues

diff --git a/src/conversions/chrono.rs b/src/conversions/chrono.rs
@@ -47,6 +47,7 @@ use crate::sync::GILOnceCell;
 use crate::types::any::PyAnyMethods;
 #[cfg(not(Py_LIMITED_API))]
 use crate::types::datetime::timezone_from_offset;
+use crate::types::PyNone;
 #[cfg(not(Py_LIMITED_API))]
 use crate::types::{
     timezone_utc, PyDate, PyDateAccess, PyDateTime, PyDelta, PyDeltaAccess, PyTime, PyTimeAccess,
@@ -553,12 +554,12 @@ impl FromPyObject<'_> for FixedOffset {
         #[cfg(Py_LIMITED_API)]
         check_type(ob, &DatetimeTypes::get(ob.py()).tzinfo, "PyTzInfo")?;
 
-        // Passing `()` (so Python's None) to the `utcoffset` function will only
+        // Passing Python's None to the `utcoffset` function will only
         // work for timezones defined as fixed offsets in Python.
         // Any other timezone would require a datetime as the parameter, and return
         // None if the datetime is not provided.
         // Trying to convert None to a PyDelta in the next line will then fail.
-        let py_timedelta = ob.call_method1("utcoffset", ((),))?;
+        let py_timedelta = ob.call_method1("utcoffset", (PyNone::get(ob.py()),))?;
         if py_timedelta.is_none() {
             return Err(PyTypeError::new_err(format!(
                 "{:?} is not a fixed offset timezone",
@@ -812,7 +813,7 @@ fn timezone_utc(py: Python<'_>) -> Bound<'_, PyAny> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{types::PyTuple, Py};
+    use crate::types::PyTuple;
     use std::{cmp::Ordering, panic};
 
     #[test]
@@ -1323,11 +1324,11 @@ mod tests {
         })
     }
 
-    fn new_py_datetime_ob<'py>(
-        py: Python<'py>,
-        name: &str,
-        args: impl IntoPy<Py<PyTuple>>,
-    ) -> Bound<'py, PyAny> {
+    fn new_py_datetime_ob<'py, A>(py: Python<'py>, name: &str, args: A) -> Bound<'py, PyAny>
+    where
+        A: IntoPyObject<'py, Target = PyTuple>,
+        A::Error: Into<PyErr>,
+    {
         py.import("datetime")
             .unwrap()
             .getattr(name)

diff --git a/src/conversions/std/num.rs b/src/conversions/std/num.rs
@@ -46,6 +46,16 @@ macro_rules! int_fits_larger_int {
             }
         }
 
+        impl<'py> IntoPyObject<'py> for &$rust_type {
+            type Target = PyInt;
+            type Output = Bound<'py, Self::Target>;
+            type Error = Infallible;
+
+            fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
+                (*self).into_pyobject(py)
+            }
+        }
+
         impl FromPyObject<'_> for $rust_type {
             fn extract_bound(obj: &Bound<'_, PyAny>) -> PyResult<Self> {
                 let val: $larger_type = obj.extract()?;

diff --git a/src/instance.rs b/src/instance.rs
@@ -1,3 +1,4 @@
+use crate::conversion::IntoPyObject;
 use crate::err::{self, PyErr, PyResult};
 use crate::impl_::pycell::PyClassObject;
 use crate::internal_tricks::ptr_from_ref;
@@ -1426,9 +1427,10 @@ impl<T> Py<T> {
     /// #    version(sys, py).unwrap();
     /// # });
     /// ```
-    pub fn getattr<N>(&self, py: Python<'_>, attr_name: N) -> PyResult<PyObject>
+    pub fn getattr<'py, N>(&self, py: Python<'py>, attr_name: N) -> PyResult<PyObject>
     where
-        N: IntoPy<Py<PyString>>,
+        N: IntoPyObject<'py, Target = PyString>,
+        N::Error: Into<PyErr>,
     {
         self.bind(py).as_any().getattr(attr_name).map(Bound::unbind)
     }
@@ -1455,32 +1457,40 @@ impl<T> Py<T> {
     /// #    set_answer(ob, py).unwrap();
     /// # });
     /// ```
-    pub fn setattr<N, V>(&self, py: Python<'_>, attr_name: N, value: V) -> PyResult<()>
+    pub fn setattr<'py, N, V>(&self, py: Python<'py>, attr_name: N, value: V) -> PyResult<()>
     where
-        N: IntoPy<Py<PyString>>,
-        V: IntoPy<Py<PyAny>>,
+        N: IntoPyObject<'py, Target = PyString>,
+        V: IntoPyObject<'py>,
+        N::Error: Into<PyErr>,
+        V::Error: Into<PyErr>,
     {
-        self.bind(py)
-            .as_any()
-            .setattr(attr_name, value.into_py(py).into_bound(py))
+        self.bind(py).as_any().setattr(attr_name, value)
     }
 
     /// Calls the object.
     ///
     /// This is equivalent to the Python expression `self(*args, **kwargs)`.
-    pub fn call_bound(
+    pub fn call_bound<'py, N>(
         &self,
-        py: Python<'_>,
-        args: impl IntoPy<Py<PyTuple>>,
-        kwargs: Option<&Bound<'_, PyDict>>,
-    ) -> PyResult<PyObject> {
+        py: Python<'py>,
+        args: N,
+        kwargs: Option<&Bound<'py, PyDict>>,
+    ) -> PyResult<PyObject>
+    where
+        N: IntoPyObject<'py, Target = PyTuple>,
+        N::Error: Into<PyErr>,
+    {
         self.bind(py).as_any().call(args, kwargs).map(Bound::unbind)
     }
 
     /// Calls the object with only positional arguments.
     ///
     /// This is equivalent to the Python expression `self(*args)`.
-    pub fn call1(&self, py: Python<'_>, args: impl IntoPy<Py<PyTuple>>) -> PyResult<PyObject> {
+    pub fn call1<'py, N>(&self, py: Python<'py>, args: N) -> PyResult<PyObject>
+    where
+        N: IntoPyObject<'py, Target = PyTuple>,
+        N::Error: Into<PyErr>,
+    {
         self.bind(py).as_any().call1(args).map(Bound::unbind)
     }
 
@@ -1497,16 +1507,18 @@ impl<T> Py<T> {
     ///
     /// To avoid repeated temporary allocations of Python strings, the [`intern!`](crate::intern)
     /// macro can be used to intern `name`.
-    pub fn call_method_bound<N, A>(
+    pub fn call_method_bound<'py, N, A>(
         &self,
-        py: Python<'_>,
+        py: Python<'py>,
         name: N,
         args: A,
         kwargs: Option<&Bound<'_, PyDict>>,
     ) -> PyResult<PyObject>
     where
-        N: IntoPy<Py<PyString>>,
-        A: IntoPy<Py<PyTuple>>,
+        N: IntoPyObject<'py, Target = PyString>,
+        A: IntoPyObject<'py, Target = PyTuple>,
+        N::Error: Into<PyErr>,
+        A::Error: Into<PyErr>,
     {
         self.bind(py)
             .as_any()
@@ -1520,10 +1532,12 @@ impl<T> Py<T> {
     ///
     /// To avoid repeated temporary allocations of Python strings, the [`intern!`](crate::intern)
     /// macro can be used to intern `name`.
-    pub fn call_method1<N, A>(&self, py: Python<'_>, name: N, args: A) -> PyResult<PyObject>
+    pub fn call_method1<'py, N, A>(&self, py: Python<'py>, name: N, args: A) -> PyResult<PyObject>
     where
-        N: IntoPy<Py<PyString>>,
-        A: IntoPy<Py<PyTuple>>,
+        N: IntoPyObject<'py, Target = PyString>,
+        A: IntoPyObject<'py, Target = PyTuple>,
+        N::Error: Into<PyErr>,
+        A::Error: Into<PyErr>,
     {
         self.bind(py)
             .as_any()
@@ -1537,9 +1551,10 @@ impl<T> Py<T> {
     ///
     /// To avoid repeated temporary allocations of Python strings, the [`intern!`](crate::intern)
     /// macro can be used to intern `name`.
-    pub fn call_method0<N>(&self, py: Python<'_>, name: N) -> PyResult<PyObject>
+    pub fn call_method0<'py, N>(&self, py: Python<'py>, name: N) -> PyResult<PyObject>
     where
-        N: IntoPy<Py<PyString>>,
+        N: IntoPyObject<'py, Target = PyString>,
+        N::Error: Into<PyErr>,
     {
         self.bind(py).as_any().call_method0(name).map(Bound::unbind)
     }

diff --git a/src/prelude.rs b/src/prelude.rs
@@ -8,7 +8,7 @@
 //! use pyo3::prelude::*;
 //! ```
 
-pub use crate::conversion::{FromPyObject, IntoPy, ToPyObject};
+pub use crate::conversion::{FromPyObject, IntoPy, IntoPyObject, ToPyObject};
 pub use crate::err::{PyErr, PyResult};
 pub use crate::instance::{Borrowed, Bound, Py, PyObject};
 pub use crate::marker::Python;