Skip to content

Commit

Permalink
deprecate PyWeakRefMethods::get_object (#4597)
Browse files Browse the repository at this point in the history 
* deprecate `PyWeakRefMethods::get_object`

* newsfragment

* fixup example
  • Loading branch information
@davidhewitt
davidhewitt authored Oct 10, 2024
1 parent 2125c0e commit 446676d
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 43 deletions.
1 change: 1 addition & 0 deletions newsfragments/4597.changed.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate `PyWeakrefMethods::get_option`.
36 changes: 19 additions & 17 deletions src/types/weakref/anyref.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::types::{
any::{PyAny, PyAnyMethods},
PyNone,
};
use crate::{ffi, Bound};
use crate::{ffi, Bound, Python};

/// Represents any Python `weakref` reference.
///
Expand Down Expand Up @@ -315,20 +315,12 @@ pub trait PyWeakrefMethods<'py>: crate::sealed::Sealed {
///
/// # Panics
/// This function panics is the current object is invalid.
/// If used propperly this is never the case. (NonNull and actually a weakref type)
/// If used properly this is never the case. (NonNull and actually a weakref type)
///
/// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef
/// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType
/// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let object = self.get_object();

if object.is_none() {
None
} else {
Some(object)
}
}
fn upgrade(&self) -> Option<Bound<'py, PyAny>>;

/// Retrieve to a Bound object pointed to by the weakref.
///
Expand All @@ -346,6 +338,7 @@ pub trait PyWeakrefMethods<'py>: crate::sealed::Sealed {
all(feature = "macros", not(all(Py_LIMITED_API, not(Py_3_9)))),
doc = "```rust"
)]
/// #![allow(deprecated)]
/// use pyo3::prelude::*;
/// use pyo3::types::PyWeakrefReference;
///
Expand Down Expand Up @@ -386,18 +379,25 @@ pub trait PyWeakrefMethods<'py>: crate::sealed::Sealed {
/// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef
/// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType
/// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref
fn get_object(&self) -> Bound<'py, PyAny>;
#[deprecated(since = "0.23.0", note = "Use `upgrade` instead")]
fn get_object(&self) -> Bound<'py, PyAny> {
self.upgrade().unwrap_or_else(|| {
// Safety: upgrade() returns `Bound<'py, PyAny>` with a lifetime `'py` if it exists, we
// can safely assume the same lifetime here.
PyNone::get(unsafe { Python::assume_gil_acquired() })
.to_owned()
.into_any()
})
}
}

impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakref> {
fn get_object(&self) -> Bound<'py, PyAny> {
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let mut obj: *mut ffi::PyObject = std::ptr::null_mut();
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref' weak reference instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
// Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is
// a valid strong reference.
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) },
0 => None,
1..=std::os::raw::c_int::MAX => Some(unsafe { obj.assume_owned_unchecked(self.py()) }),
}
}
}
Expand Down Expand Up @@ -547,6 +547,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
fn inner(
create_reference: impl for<'py> FnOnce(
Expand Down Expand Up @@ -698,6 +699,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
fn inner(
create_reference: impl for<'py> FnOnce(
Expand Down
33 changes: 17 additions & 16 deletions src/types/weakref/proxy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::err::PyResult;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::py_result_ext::PyResultExt;
use crate::type_object::PyTypeCheck;
use crate::types::{any::PyAny, PyNone};
use crate::types::any::PyAny;
use crate::{ffi, Bound, BoundObject, IntoPyObject};

use super::PyWeakrefMethods;
Expand Down Expand Up @@ -190,14 +190,12 @@ impl PyWeakrefProxy {
}

impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefProxy> {
fn get_object(&self) -> Bound<'py, PyAny> {
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let mut obj: *mut ffi::PyObject = std::ptr::null_mut();
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ProxyType' (or `weakref.CallableProxyType`) instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
// Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is
// a valid strong reference.
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) },
0 => None,
1..=std::os::raw::c_int::MAX => Some(unsafe { obj.assume_owned_unchecked(self.py()) }),
}
}
}
Expand Down Expand Up @@ -283,7 +281,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));

#[cfg(not(Py_LIMITED_API))]
assert_eq!(
Expand Down Expand Up @@ -311,7 +309,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -426,11 +424,11 @@ mod tests {
let object = class.call0()?;
let reference = PyWeakrefProxy::new(&object)?;

assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());

Ok(())
})
Expand All @@ -454,7 +452,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(
reference.get_type().to_string(),
Expand Down Expand Up @@ -484,7 +482,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -584,6 +582,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let object = Py::new(py, WeakrefablePyClass {})?;
Expand Down Expand Up @@ -633,7 +632,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);

Expand All @@ -650,7 +649,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -757,6 +756,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let class = get_type(py)?;
Expand Down Expand Up @@ -798,7 +798,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);

Expand All @@ -818,7 +818,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -916,6 +916,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let object = Py::new(py, WeakrefablePyClass {})?;
Expand Down
20 changes: 10 additions & 10 deletions src/types/weakref/reference.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::err::PyResult;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::py_result_ext::PyResultExt;
use crate::types::{any::PyAny, PyNone};
use crate::types::any::PyAny;
use crate::{ffi, Bound, BoundObject, IntoPyObject};

#[cfg(any(PyPy, GraalPy, Py_LIMITED_API))]
Expand Down Expand Up @@ -199,14 +199,12 @@ impl PyWeakrefReference {
}

impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefReference> {
fn get_object(&self) -> Bound<'py, PyAny> {
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let mut obj: *mut ffi::PyObject = std::ptr::null_mut();
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ReferenceType' instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
// Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is
// a valid strong reference.
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) },
0 => None,
1..=std::os::raw::c_int::MAX => Some(unsafe { obj.assume_owned_unchecked(self.py()) }),
}
}
}
Expand Down Expand Up @@ -278,7 +276,7 @@ mod tests {
let reference = PyWeakrefReference::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));

#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);
Expand All @@ -297,7 +295,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.getattr("__class__")?.to_string(), CLASS_NAME);
check_repr(&reference, None)?;
Expand Down Expand Up @@ -397,6 +395,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let class = get_type(py)?;
Expand Down Expand Up @@ -433,7 +432,7 @@ mod tests {
let reference = PyWeakrefReference::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);

Expand All @@ -450,7 +449,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.getattr("__class__")?.to_string(), CLASS_NAME);
check_repr(&reference, None)?;
Expand Down Expand Up @@ -541,6 +540,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let object = Py::new(py, WeakrefablePyClass {})?;
Expand Down

0 comments on commit 446676d

Please sign in to comment.