From a93900686ecd6292cecbd7d55db1a7ae691d7bcd Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Tue, 20 Feb 2024 07:10:45 +0000 Subject: [PATCH] Deprecate Py::into_ref (#3867) * Migrate into_ref calls to Bound api * Mark Py::into_ref as deprecated --- guide/src/types.md | 3 +++ src/conversions/chrono.rs | 2 +- src/exceptions.rs | 5 +++-- src/impl_/pymodule.rs | 4 ++-- src/instance.rs | 8 ++++++++ src/marker.rs | 2 ++ src/types/any.rs | 20 ++++++++++---------- src/types/traceback.rs | 2 +- tests/test_proto_methods.rs | 32 +++++++++++++++++++++----------- 9 files changed, 51 insertions(+), 27 deletions(-) diff --git a/guide/src/types.md b/guide/src/types.md index 882baddb..1a926f43 100644 --- a/guide/src/types.md +++ b/guide/src/types.md @@ -92,6 +92,7 @@ For a `&PyAny` object reference `any` where the underlying object is a `#[pyclas # use pyo3::prelude::*; # #[pyclass] #[derive(Clone)] struct MyClass { } # Python::with_gil(|py| -> PyResult<()> { +# #[allow(deprecated)] let obj: &PyAny = Py::new(py, MyClass {})?.into_ref(py); // To &PyCell with PyAny::downcast @@ -182,6 +183,7 @@ let _: &PyList = list.as_ref(py); # let list_clone = list.clone(); // Because `.into_ref()` will consume `list`. // To &PyList with Py::into_ref() (moves the pointer into PyO3's object storage) +# #[allow(deprecated)] let _: &PyList = list.into_ref(py); # let list = list_clone; @@ -204,6 +206,7 @@ let _: &PyCell = my_class.as_ref(py); # let my_class_clone = my_class.clone(); // Because `.into_ref()` will consume `my_class`. // To &PyCell with Py::into_ref() (moves the pointer into PyO3's object storage) +# #[allow(deprecated)] let _: &PyCell = my_class.into_ref(py); # let my_class = my_class_clone.clone(); diff --git a/src/conversions/chrono.rs b/src/conversions/chrono.rs index c4fa1068..3aa6dbeb 100644 --- a/src/conversions/chrono.rs +++ b/src/conversions/chrono.rs @@ -636,7 +636,7 @@ mod tests { // Test that if a user tries to convert a python's timezone aware datetime into a naive // one, the conversion fails. Python::with_gil(|py| { - let none = py.None().into_ref(py); + let none = py.None().into_bound(py); assert_eq!( none.extract::().unwrap_err().to_string(), "TypeError: 'NoneType' object cannot be converted to 'PyDelta'" diff --git a/src/exceptions.rs b/src/exceptions.rs index cf471053..70809448 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -1008,7 +1008,7 @@ mod tests { .run_bound("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") .into_value(py) - .into_ref(py); + .into_bound(py); assert_eq!( format!("{:?}", exc), exc.repr().unwrap().extract::().unwrap() @@ -1023,7 +1023,7 @@ mod tests { .run_bound("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") .into_value(py) - .into_ref(py); + .into_bound(py); assert_eq!( exc.to_string(), exc.str().unwrap().extract::().unwrap() @@ -1036,6 +1036,7 @@ mod tests { use std::error::Error; Python::with_gil(|py| { + #[allow(deprecated)] let exc = py .run_bound( "raise Exception('banana') from TypeError('peach')", diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index ff722855..7103f8b3 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -137,7 +137,7 @@ impl ModuleDef { mod tests { use std::sync::atomic::{AtomicBool, Ordering}; - use crate::{types::PyModule, PyResult, Python}; + use crate::{types::any::PyAnyMethods, types::PyModule, PyResult, Python}; use super::{ModuleDef, ModuleInitializer}; @@ -154,7 +154,7 @@ mod tests { ) }; Python::with_gil(|py| { - let module = MODULE_DEF.make_module(py).unwrap().into_ref(py); + let module = MODULE_DEF.make_module(py).unwrap().into_bound(py); assert_eq!( module .getattr("__name__") diff --git a/src/instance.rs b/src/instance.rs index 5049a66b..3f19637e 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -958,9 +958,17 @@ where /// // This reference's lifetime is determined by `py`'s lifetime. /// // Because that originates from outside this function, /// // this return value is allowed. + /// # #[allow(deprecated)] /// obj.into_ref(py) /// } /// ``` + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "part of the deprecated GIL Ref API; to migrate use `obj.into_bound(py)` instead of `obj.into_ref(py)`" + ) + )] pub fn into_ref(self, py: Python<'_>) -> &T::AsRefTarget { unsafe { py.from_owned_ptr(self.into_ptr()) } } diff --git a/src/marker.rs b/src/marker.rs index 8a52bbaf..29583e8e 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -853,6 +853,7 @@ impl<'py> Python<'py> { where T: PyTypeCheck, { + #[allow(deprecated)] obj.into_ref(self).downcast() } @@ -873,6 +874,7 @@ impl<'py> Python<'py> { where T: HasPyGilRef, { + #[allow(deprecated)] obj.into_ref(self).downcast_unchecked() } diff --git a/src/types/any.rs b/src/types/any.rs index 1cb6bb77..4d371764 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -259,8 +259,8 @@ impl PyAny { /// /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| -> PyResult<()> { - /// let a: &PyInt = 0_u8.into_py(py).into_ref(py).downcast()?; - /// let b: &PyInt = 42_u8.into_py(py).into_ref(py).downcast()?; + /// let a: Bound<'_, PyInt> = 0_u8.into_py(py).into_bound(py).downcast_into()?; + /// let b: Bound<'_, PyInt> = 42_u8.into_py(py).into_bound(py).downcast_into()?; /// assert!(a.rich_compare(b, CompareOp::Le)?.is_truthy()?); /// Ok(()) /// })?; @@ -715,11 +715,11 @@ impl PyAny { /// } /// /// Python::with_gil(|py| { - /// let class: &PyAny = Py::new(py, Class { i: 0 }).unwrap().into_ref(py); + /// let class = Py::new(py, Class { i: 0 }).unwrap().into_bound(py).into_any(); /// - /// let class_cell: &PyCell = class.downcast()?; + /// let class_bound: &Bound<'_, Class> = class.downcast()?; /// - /// class_cell.borrow_mut().i += 1; + /// class_bound.borrow_mut().i += 1; /// /// // Alternatively you can get a `PyRefMut` directly /// let class_ref: PyRefMut<'_, Class> = class.extract()?; @@ -1112,8 +1112,8 @@ pub trait PyAnyMethods<'py> { /// /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| -> PyResult<()> { - /// let a: &PyInt = 0_u8.into_py(py).into_ref(py).downcast()?; - /// let b: &PyInt = 42_u8.into_py(py).into_ref(py).downcast()?; + /// let a: Bound<'_, PyInt> = 0_u8.into_py(py).into_bound(py).downcast_into()?; + /// let b: Bound<'_, PyInt> = 42_u8.into_py(py).into_bound(py).downcast_into()?; /// assert!(a.rich_compare(b, CompareOp::Le)?.is_truthy()?); /// Ok(()) /// })?; @@ -1543,11 +1543,11 @@ pub trait PyAnyMethods<'py> { /// } /// /// Python::with_gil(|py| { - /// let class: &PyAny = Py::new(py, Class { i: 0 }).unwrap().into_ref(py); + /// let class = Py::new(py, Class { i: 0 }).unwrap().into_bound(py).into_any(); /// - /// let class_cell: &PyCell = class.downcast()?; + /// let class_bound: &Bound<'_, Class> = class.downcast()?; /// - /// class_cell.borrow_mut().i += 1; + /// class_bound.borrow_mut().i += 1; /// /// // Alternatively you can get a `PyRefMut` directly /// let class_ref: PyRefMut<'_, Class> = class.extract()?; diff --git a/src/types/traceback.rs b/src/types/traceback.rs index 33a1ec7c..24b935e2 100644 --- a/src/types/traceback.rs +++ b/src/types/traceback.rs @@ -173,7 +173,7 @@ def f(): let f = locals.get_item("f").unwrap().unwrap(); let err = f.call0().unwrap_err(); let traceback = err.traceback_bound(py).unwrap(); - let err_object = err.clone_ref(py).into_py(py).into_ref(py); + let err_object = err.clone_ref(py).into_py(py).into_bound(py); assert!(err_object.getattr("__traceback__").unwrap().is(&traceback)); }) diff --git a/tests/test_proto_methods.rs b/tests/test_proto_methods.rs index 15c8a0e0..c57ca6a3 100644 --- a/tests/test_proto_methods.rs +++ b/tests/test_proto_methods.rs @@ -64,8 +64,8 @@ impl ExampleClass { } } -fn make_example(py: Python<'_>) -> &PyCell { - Py::new( +fn make_example(py: Python<'_>) -> Bound<'_, ExampleClass> { + Bound::new( py, ExampleClass { value: 5, @@ -73,7 +73,6 @@ fn make_example(py: Python<'_>) -> &PyCell { }, ) .unwrap() - .into_ref(py) } #[test] @@ -82,6 +81,7 @@ fn test_getattr() { let example_py = make_example(py); assert_eq!( example_py + .as_any() .getattr("value") .unwrap() .extract::() @@ -90,6 +90,7 @@ fn test_getattr() { ); assert_eq!( example_py + .as_any() .getattr("special_custom_attr") .unwrap() .extract::() @@ -97,6 +98,7 @@ fn test_getattr() { 20, ); assert!(example_py + .as_any() .getattr("other_attr") .unwrap_err() .is_instance_of::(py)); @@ -107,9 +109,13 @@ fn test_getattr() { fn test_setattr() { Python::with_gil(|py| { let example_py = make_example(py); - example_py.setattr("special_custom_attr", 15).unwrap(); + example_py + .as_any() + .setattr("special_custom_attr", 15) + .unwrap(); assert_eq!( example_py + .as_any() .getattr("special_custom_attr") .unwrap() .extract::() @@ -123,8 +129,12 @@ fn test_setattr() { fn test_delattr() { Python::with_gil(|py| { let example_py = make_example(py); - example_py.delattr("special_custom_attr").unwrap(); - assert!(example_py.getattr("special_custom_attr").unwrap().is_none()); + example_py.as_any().delattr("special_custom_attr").unwrap(); + assert!(example_py + .as_any() + .getattr("special_custom_attr") + .unwrap() + .is_none()); }) } @@ -132,7 +142,7 @@ fn test_delattr() { fn test_str() { Python::with_gil(|py| { let example_py = make_example(py); - assert_eq!(example_py.str().unwrap().to_str().unwrap(), "5"); + assert_eq!(example_py.as_any().str().unwrap().to_cow().unwrap(), "5"); }) } @@ -141,7 +151,7 @@ fn test_repr() { Python::with_gil(|py| { let example_py = make_example(py); assert_eq!( - example_py.repr().unwrap().to_str().unwrap(), + example_py.as_any().repr().unwrap().to_cow().unwrap(), "ExampleClass(value=5)" ); }) @@ -151,7 +161,7 @@ fn test_repr() { fn test_hash() { Python::with_gil(|py| { let example_py = make_example(py); - assert_eq!(example_py.hash().unwrap(), 5); + assert_eq!(example_py.as_any().hash().unwrap(), 5); }) } @@ -159,9 +169,9 @@ fn test_hash() { fn test_bool() { Python::with_gil(|py| { let example_py = make_example(py); - assert!(example_py.is_truthy().unwrap()); + assert!(example_py.as_any().is_truthy().unwrap()); example_py.borrow_mut().value = 0; - assert!(!example_py.is_truthy().unwrap()); + assert!(!example_py.as_any().is_truthy().unwrap()); }) }