migrate some final `FromPyObject` implementations to the `Bound` API (#3869)

* update `Py::extract` to use `extract_bound`

* update docstring of `FromPyObject`

* move `Option` conversions to new module & update

* move `Cell` conversions to new module & update

src/conversion.rs

@@ -8,7 +8,6 @@ use crate::types::PyTuple;
 use crate::{
     ffi, gil, Bound, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python,
 };
-use std::cell::Cell;
 use std::ptr::NonNull;
 
 /// Returns a borrowed pointer to a Python object.
@@ -63,18 +62,6 @@ pub unsafe trait AsPyPointer {
     fn as_ptr(&self) -> *mut ffi::PyObject;
 }
 
-/// Convert `None` into a null pointer.
-unsafe impl<T> AsPyPointer for Option<T>
-where
-    T: AsPyPointer,
-{
-    #[inline]
-    fn as_ptr(&self) -> *mut ffi::PyObject {
-        self.as_ref()
-            .map_or_else(std::ptr::null_mut, |t| t.as_ptr())
-    }
-}
-
 /// Conversion trait that allows various objects to be converted into `PyObject`.
 pub trait ToPyObject {
     /// Converts self into a Python object.
@@ -180,7 +167,7 @@ pub trait IntoPy<T>: Sized {
 /// Extract a type from a Python object.
 ///
 ///
-/// Normal usage is through the `extract` methods on [`Py`] and  [`PyAny`], which forward to this trait.
+/// Normal usage is through the `extract` methods on [`Bound`] and [`Py`], which forward to this trait.
 ///
 /// # Examples
 ///
@@ -190,30 +177,32 @@ pub trait IntoPy<T>: Sized {
 ///
 /// # fn main() -> PyResult<()> {
 /// Python::with_gil(|py| {
-///     let obj: Py<PyString> = PyString::new_bound(py, "blah").into();
-///
-///     // Straight from an owned reference
-///     let s: &str = obj.extract(py)?;
+///     // Calling `.extract()` on a `Bound` smart pointer
+///     let obj: Bound<'_, PyString> = PyString::new_bound(py, "blah");
+///     let s: &str = obj.extract()?;
 /// #   assert_eq!(s, "blah");
 ///
-///     // Or from a borrowed reference
-///     let obj: &PyString = obj.as_ref(py);
-///     let s: &str = obj.extract()?;
+///     // Calling `.extract(py)` on a `Py` smart pointer
+///     let obj: Py<PyString> = obj.unbind();
+///     let s: &str = obj.extract(py)?;
 /// #   assert_eq!(s, "blah");
 /// #   Ok(())
 /// })
 /// # }
 /// ```
 ///
-/// Note: depending on the implementation, the lifetime of the extracted result may
-/// depend on the lifetime of the `obj` or the `prepared` variable.
-///
-/// For example, when extracting `&str` from a Python byte string, the resulting string slice will
-/// point to the existing string data (lifetime: `'py`).
-/// On the other hand, when extracting `&str` from a Python Unicode string, the preparation step
-/// will convert the string to UTF-8, and the resulting string slice will have lifetime `'prepared`.
-/// Since which case applies depends on the runtime type of the Python object,
-/// both the `obj` and `prepared` variables must outlive the resulting string slice.
+// /// FIXME: until `FromPyObject` can pick up a second lifetime, the below commentary is no longer
+// /// true. Update and restore this documentation at that time.
+// ///
+// /// Note: depending on the implementation, the lifetime of the extracted result may
+// /// depend on the lifetime of the `obj` or the `prepared` variable.
+// ///
+// /// For example, when extracting `&str` from a Python byte string, the resulting string slice will
+// /// point to the existing string data (lifetime: `'py`).
+// /// On the other hand, when extracting `&str` from a Python Unicode string, the preparation step
+// /// will convert the string to UTF-8, and the resulting string slice will have lifetime `'prepared`.
+// /// Since which case applies depends on the runtime type of the Python object,
+// /// both the `obj` and `prepared` variables must outlive the resulting string slice.
 ///
 /// During the migration of PyO3 from the "GIL Refs" API to the `Bound<T>` smart pointer, this trait
 /// has two methods `extract` and `extract_bound` which are defaulted to call each other. To avoid
@@ -258,27 +247,6 @@ impl<T: ?Sized + ToPyObject> ToPyObject for &'_ T {
     }
 }
 
-/// `Option::Some<T>` is converted like `T`.
-/// `Option::None` is converted to Python `None`.
-impl<T> ToPyObject for Option<T>
-where
-    T: ToPyObject,
-{
-    fn to_object(&self, py: Python<'_>) -> PyObject {
-        self.as_ref()
-            .map_or_else(|| py.None(), |val| val.to_object(py))
-    }
-}
-
-impl<T> IntoPy<PyObject> for Option<T>
-where
-    T: IntoPy<PyObject>,
-{
-    fn into_py(self, py: Python<'_>) -> PyObject {
-        self.map_or_else(|| py.None(), |val| val.into_py(py))
-    }
-}
-
 impl IntoPy<PyObject> for &'_ PyAny {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
@@ -296,24 +264,6 @@ where
     }
 }
 
-impl<T: Copy + ToPyObject> ToPyObject for Cell<T> {
-    fn to_object(&self, py: Python<'_>) -> PyObject {
-        self.get().to_object(py)
-    }
-}
-
-impl<T: Copy + IntoPy<PyObject>> IntoPy<PyObject> for Cell<T> {
-    fn into_py(self, py: Python<'_>) -> PyObject {
-        self.get().into_py(py)
-    }
-}
-
-impl<'py, T: FromPyObject<'py>> FromPyObject<'py> for Cell<T> {
-    fn extract(ob: &'py PyAny) -> PyResult<Self> {
-        T::extract(ob).map(Cell::new)
-    }
-}
-
 impl<'py, T> FromPyObject<'py> for &'py PyCell<T>
 where
     T: PyClass,
@@ -353,19 +303,6 @@ where
     }
 }
 
-impl<'py, T> FromPyObject<'py> for Option<T>
-where
-    T: FromPyObject<'py>,
-{
-    fn extract(obj: &'py PyAny) -> PyResult<Self> {
-        if obj.as_ptr() == unsafe { ffi::Py_None() } {
-            Ok(None)
-        } else {
-            T::extract(obj).map(Some)
-        }
-    }
-}
-
 /// Trait implemented by Python object types that allow a checked downcast.
 /// If `T` implements `PyTryFrom`, we can convert `&PyAny` to `&T`.
 ///
@@ -593,8 +530,6 @@ mod test_no_clone {}
 
 #[cfg(test)]
 mod tests {
-    use crate::{PyObject, Python};
-
     #[allow(deprecated)]
     mod deprecated {
         use super::super::PyTryFrom;
@@ -638,22 +573,4 @@ mod tests {
             });
         }
     }
-
-    #[test]
-    fn test_option_as_ptr() {
-        Python::with_gil(|py| {
-            use crate::AsPyPointer;
-            let mut option: Option<PyObject> = None;
-            assert_eq!(option.as_ptr(), std::ptr::null_mut());
-
-            let none = py.None();
-            option = Some(none.clone());
-
-            let ref_cnt = none.get_refcnt(py);
-            assert_eq!(option.as_ptr(), none.as_ptr());
-
-            // Ensure ref count not changed by as_ptr call
-            assert_eq!(none.get_refcnt(py), ref_cnt);
-        });
-    }
 }

src/conversions/std/cell.rs

@@ -0,0 +1,24 @@
+use std::cell::Cell;
+
+use crate::{
+    types::any::PyAnyMethods, Bound, FromPyObject, IntoPy, PyAny, PyObject, PyResult, Python,
+    ToPyObject,
+};
+
+impl<T: Copy + ToPyObject> ToPyObject for Cell<T> {
+    fn to_object(&self, py: Python<'_>) -> PyObject {
+        self.get().to_object(py)
+    }
+}
+
+impl<T: Copy + IntoPy<PyObject>> IntoPy<PyObject> for Cell<T> {
+    fn into_py(self, py: Python<'_>) -> PyObject {
+        self.get().into_py(py)
+    }
+}
+
+impl<'py, T: FromPyObject<'py>> FromPyObject<'py> for Cell<T> {
+    fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
+        ob.extract().map(Cell::new)
+    }
+}

src/conversions/std/mod.rs

@@ -1,7 +1,9 @@
 mod array;
+mod cell;
 mod ipaddr;
 mod map;
 mod num;
+mod option;
 mod osstr;
 mod path;
 mod set;

src/conversions/std/option.rs

@@ -0,0 +1,73 @@
+use crate::{
+    ffi, types::any::PyAnyMethods, AsPyPointer, Bound, FromPyObject, IntoPy, PyAny, PyObject,
+    PyResult, Python, ToPyObject,
+};
+
+/// `Option::Some<T>` is converted like `T`.
+/// `Option::None` is converted to Python `None`.
+impl<T> ToPyObject for Option<T>
+where
+    T: ToPyObject,
+{
+    fn to_object(&self, py: Python<'_>) -> PyObject {
+        self.as_ref()
+            .map_or_else(|| py.None(), |val| val.to_object(py))
+    }
+}
+
+impl<T> IntoPy<PyObject> for Option<T>
+where
+    T: IntoPy<PyObject>,
+{
+    fn into_py(self, py: Python<'_>) -> PyObject {
+        self.map_or_else(|| py.None(), |val| val.into_py(py))
+    }
+}
+
+impl<'py, T> FromPyObject<'py> for Option<T>
+where
+    T: FromPyObject<'py>,
+{
+    fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult<Self> {
+        if obj.is_none() {
+            Ok(None)
+        } else {
+            obj.extract().map(Some)
+        }
+    }
+}
+
+/// Convert `None` into a null pointer.
+unsafe impl<T> AsPyPointer for Option<T>
+where
+    T: AsPyPointer,
+{
+    #[inline]
+    fn as_ptr(&self) -> *mut ffi::PyObject {
+        self.as_ref()
+            .map_or_else(std::ptr::null_mut, |t| t.as_ptr())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::{PyObject, Python};
+
+    #[test]
+    fn test_option_as_ptr() {
+        Python::with_gil(|py| {
+            use crate::AsPyPointer;
+            let mut option: Option<PyObject> = None;
+            assert_eq!(option.as_ptr(), std::ptr::null_mut());
+
+            let none = py.None();
+            option = Some(none.clone());
+
+            let ref_cnt = none.get_refcnt(py);
+            assert_eq!(option.as_ptr(), none.as_ptr());
+
+            // Ensure ref count not changed by as_ptr call
+            assert_eq!(none.get_refcnt(py), ref_cnt);
+        });
+    }
+}

src/instance.rs

@@ -1250,11 +1250,11 @@ impl<T> Py<T> {
     /// Extracts some type from the Python object.
     ///
     /// This is a wrapper function around `FromPyObject::extract()`.
-    pub fn extract<'py, D>(&'py self, py: Python<'py>) -> PyResult<D>
+    pub fn extract<'py, D>(&self, py: Python<'py>) -> PyResult<D>
     where
         D: FromPyObject<'py>,
     {
-        FromPyObject::extract(unsafe { py.from_borrowed_ptr(self.as_ptr()) })
+        self.bind(py).as_any().extract()
     }
 
     /// Retrieves an attribute value.