Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature gate deprecated APIs for Py #4142

Merged
merged 1 commit into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions guide/src/memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,11 @@ What happens to the memory when the last `Py<PyAny>` is dropped and its
reference count reaches zero? It depends whether or not we are holding the GIL.

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
# #[cfg(feature = "gil-refs")]
Python::with_gil(|py| -> PyResult<()> {
#[allow(deprecated)] // py.eval() is part of the GIL Refs API
let hello: Py<PyString> = py.eval("\"Hello World!\"", None, None)?.extract()?;
Expand All @@ -203,9 +205,12 @@ This example wasn't very interesting. We could have just used a GIL-bound
we are *not* holding the GIL?

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
# #[cfg(feature = "gil-refs")]
# {
let hello: Py<PyString> = Python::with_gil(|py| {
#[allow(deprecated)] // py.eval() is part of the GIL Refs API
py.eval("\"Hello World!\"", None, None)?.extract()
Expand All @@ -224,6 +229,7 @@ Python::with_gil(|py|
// Memory for `hello` is released here.
# ()
);
# }
# Ok(())
# }
```
Expand All @@ -237,9 +243,12 @@ We can avoid the delay in releasing memory if we are careful to drop the
`Py<Any>` while the GIL is held.

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
# #[cfg(feature = "gil-refs")]
# {
#[allow(deprecated)] // py.eval() is part of the GIL Refs API
let hello: Py<PyString> =
Python::with_gil(|py| py.eval("\"Hello World!\"", None, None)?.extract())?;
Expand All @@ -252,6 +261,7 @@ Python::with_gil(|py| {
}
drop(hello); // Memory released here.
});
# }
# Ok(())
# }
```
Expand All @@ -263,9 +273,12 @@ that rather than being released immediately, the memory will not be released
until the GIL is dropped.

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
# #[cfg(feature = "gil-refs")]
# {
#[allow(deprecated)] // py.eval() is part of the GIL Refs API
let hello: Py<PyString> =
Python::with_gil(|py| py.eval("\"Hello World!\"", None, None)?.extract())?;
Expand All @@ -280,6 +293,7 @@ Python::with_gil(|py| {
// Do more stuff...
// Memory released here at end of `with_gil()` closure.
});
# }
# Ok(())
# }
```
2 changes: 1 addition & 1 deletion guide/src/python-from-rust/function-calls.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ fn main() -> PyResult<()> {

<div class="warning">

During PyO3's [migration from "GIL Refs" to the `Bound<T>` smart pointer](../migration.md#migrating-from-the-gil-refs-api-to-boundt), [`Py<T>::call`]({{#PYO3_DOCS_URL}}/pyo3/struct.Py.html#method.call) is temporarily named `call_bound` (and `call_method` is temporarily `call_method_bound`).
During PyO3's [migration from "GIL Refs" to the `Bound<T>` smart pointer](../migration.md#migrating-from-the-gil-refs-api-to-boundt), `Py<T>::call` is temporarily named [`Py<T>::call_bound`]({{#PYO3_DOCS_URL}}/pyo3/struct.Py.html#method.call_bound) (and `call_method` is temporarily `call_method_bound`).

(This temporary naming is only the case for the `Py<T>` smart pointer. The methods on the `&PyAny` GIL Ref such as `call` have not been given replacements, and the methods on the `Bound<PyAny>` smart pointer such as [`Bound<PyAny>::call`]({{#PYO3_DOCS_URL}}/pyo3/types/trait.PyAnyMethods.html#tymethod.call) already use follow the newest API conventions.)

Expand Down
2 changes: 2 additions & 0 deletions guide/src/types.md
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,10 @@ let _: Py<PyList> = obj.extract()?;
For a `&PyAny` object reference `any` where the underlying object is a `#[pyclass]`:

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# #[pyclass] #[derive(Clone)] struct MyClass { }
# #[cfg(feature = "gil-refs")]
# Python::with_gil(|py| -> PyResult<()> {
#[allow(deprecated)] // into_ref is part of the deprecated GIL Refs API
let obj: &PyAny = Py::new(py, MyClass {})?.into_ref(py);
Expand Down
1 change: 1 addition & 0 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl<'a> PyDowncastError<'a> {

/// Compatibility API to convert the Bound variant `DowncastError` into the
/// gil-ref variant
#[cfg(feature = "gil-refs")]
pub(crate) fn from_downcast_err(DowncastError { from, to }: DowncastError<'a, 'a>) -> Self {
#[allow(deprecated)]
let from = unsafe { from.py().from_borrowed_ptr(from.as_ptr()) };
Expand Down
115 changes: 50 additions & 65 deletions src/instance.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::err::{self, PyDowncastError, PyErr, PyResult};
use crate::err::{self, PyErr, PyResult};
use crate::impl_::pycell::PyClassObject;
use crate::pycell::{PyBorrowError, PyBorrowMutError};
use crate::pyclass::boolean_struct::{False, True};
Expand Down Expand Up @@ -721,12 +721,12 @@ impl<T> IntoPy<PyObject> for Borrowed<'_, '_, T> {
///
/// This type does not auto-dereference to the inner object because you must prove you hold the GIL to access it.
/// Instead, call one of its methods to access the inner object:
/// - [`Py::as_ref`], to borrow a GIL-bound reference to the contained object.
/// - [`Py::bind`] or [`Py::into_bound`], to borrow a GIL-bound reference to the contained object.
/// - [`Py::borrow`], [`Py::try_borrow`], [`Py::borrow_mut`], or [`Py::try_borrow_mut`],
/// to get a (mutable) reference to a contained pyclass, using a scheme similar to std's [`RefCell`].
/// See the [`PyCell` guide entry](https://pyo3.rs/latest/class.html#pycell-and-interior-mutability)
/// See the [guide entry](https://pyo3.rs/latest/class.html#bound-and-interior-mutability)
/// for more information.
/// - You can call methods directly on `Py` with [`Py::call`], [`Py::call_method`] and friends.
/// - You can call methods directly on `Py` with [`Py::call_bound`], [`Py::call_method_bound`] and friends.
/// These require passing in the [`Python<'py>`](crate::Python) token but are otherwise similar to the corresponding
/// methods on [`PyAny`].
///
Expand Down Expand Up @@ -991,12 +991,10 @@ where
/// assert!(my_class_cell.try_borrow().is_ok());
/// });
/// ```
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `obj.bind(py)` instead of `obj.as_ref(py)`"
)
#[cfg(feature = "gil-refs")]
#[deprecated(
since = "0.21.0",
note = "use `obj.bind(py)` instead of `obj.as_ref(py)`"
)]
pub fn as_ref<'py>(&'py self, _py: Python<'py>) -> &'py T::AsRefTarget {
let any = self.as_ptr() as *const PyAny;
Expand Down Expand Up @@ -1046,12 +1044,10 @@ where
/// obj.into_ref(py)
/// }
/// ```
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `obj.into_bound(py)` instead of `obj.into_ref(py)`"
)
#[cfg(feature = "gil-refs")]
#[deprecated(
since = "0.21.0",
note = "use `obj.into_bound(py)` instead of `obj.into_ref(py)`"
)]
pub fn into_ref(self, py: Python<'_>) -> &T::AsRefTarget {
#[allow(deprecated)]
Expand Down Expand Up @@ -1464,12 +1460,10 @@ impl<T> Py<T> {
}

/// Deprecated form of [`call_bound`][Py::call_bound].
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`call` will be replaced by `call_bound` in a future PyO3 version"
)
#[cfg(feature = "gil-refs")]
#[deprecated(
since = "0.21.0",
note = "`call` will be replaced by `call_bound` in a future PyO3 version"
)]
#[inline]
pub fn call<A>(&self, py: Python<'_>, args: A, kwargs: Option<&PyDict>) -> PyResult<PyObject>
Expand Down Expand Up @@ -1506,12 +1500,10 @@ impl<T> Py<T> {
}

/// Deprecated form of [`call_method_bound`][Py::call_method_bound].
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`call_method` will be replaced by `call_method_bound` in a future PyO3 version"
)
#[cfg(feature = "gil-refs")]
#[deprecated(
since = "0.21.0",
note = "`call_method` will be replaced by `call_method_bound` in a future PyO3 version"
)]
#[inline]
pub fn call_method<N, A>(
Expand Down Expand Up @@ -1779,6 +1771,7 @@ impl<T> std::convert::From<Bound<'_, T>> for Py<T> {
}

// `&PyCell<T>` can be converted to `Py<T>`
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
impl<T> std::convert::From<&crate::PyCell<T>> for Py<T>
where
Expand Down Expand Up @@ -1844,10 +1837,7 @@ where
{
/// Extracts `Self` from the source `PyObject`.
fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
// TODO update MSRV past 1.59 and use .cloned() to make
// clippy happy
#[allow(clippy::map_clone)]
ob.downcast().map(Clone::clone).map_err(Into::into)
ob.downcast().cloned().map_err(Into::into)
}
}

Expand Down Expand Up @@ -1888,21 +1878,22 @@ pub type PyObject = Py<PyAny>;

impl PyObject {
/// Deprecated form of [`PyObject::downcast_bound`]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`PyObject::downcast` will be replaced by `PyObject::downcast_bound` in a future PyO3 version"
)
#[cfg(feature = "gil-refs")]
#[deprecated(
since = "0.21.0",
note = "`PyObject::downcast` will be replaced by `PyObject::downcast_bound` in a future PyO3 version"
)]
#[inline]
pub fn downcast<'py, T>(&'py self, py: Python<'py>) -> Result<&'py T, PyDowncastError<'py>>
pub fn downcast<'py, T>(
&'py self,
py: Python<'py>,
) -> Result<&'py T, crate::err::PyDowncastError<'py>>
where
T: PyTypeCheck<AsRefTarget = T>,
{
self.downcast_bound::<T>(py)
.map(Bound::as_gil_ref)
.map_err(PyDowncastError::from_downcast_err)
.map_err(crate::err::PyDowncastError::from_downcast_err)
}
/// Downcast this `PyObject` to a concrete Python type or pyclass.
///
Expand Down Expand Up @@ -1970,12 +1961,10 @@ impl PyObject {
/// # Safety
///
/// Callers must ensure that the type is valid or risk type confusion.
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`PyObject::downcast_unchecked` will be replaced by `PyObject::downcast_bound_unchecked` in a future PyO3 version"
)
#[cfg(feature = "gil-refs")]
#[deprecated(
since = "0.21.0",
note = "`PyObject::downcast_unchecked` will be replaced by `PyObject::downcast_bound_unchecked` in a future PyO3 version"
)]
#[inline]
pub unsafe fn downcast_unchecked<'py, T>(&'py self, py: Python<'py>) -> &T
Expand All @@ -1997,35 +1986,31 @@ impl PyObject {
}

#[cfg(test)]
#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))]
mod tests {
use super::{Bound, Py, PyObject};
use crate::types::any::PyAnyMethods;
use crate::types::{dict::IntoPyDict, PyDict, PyString};
use crate::types::{PyCapsule, PyStringMethods};
use crate::{ffi, Borrowed, PyAny, PyNativeType, PyResult, Python, ToPyObject};
use crate::{ffi, Borrowed, PyAny, PyResult, Python, ToPyObject};

#[test]
fn test_call() {
Python::with_gil(|py| {
let obj = py.get_type::<PyDict>().to_object(py);
let obj = py.get_type_bound::<PyDict>().to_object(py);

let assert_repr = |obj: &PyAny, expected: &str| {
assert_eq!(obj.repr().unwrap().to_str().unwrap(), expected);
let assert_repr = |obj: &Bound<'_, PyAny>, expected: &str| {
assert_eq!(obj.repr().unwrap().to_cow().unwrap(), expected);
};

assert_repr(obj.call0(py).unwrap().as_ref(py), "{}");
assert_repr(obj.call1(py, ()).unwrap().as_ref(py), "{}");
assert_repr(obj.call(py, (), None).unwrap().as_ref(py), "{}");
assert_repr(obj.call0(py).unwrap().bind(py), "{}");
assert_repr(obj.call1(py, ()).unwrap().bind(py), "{}");
assert_repr(obj.call_bound(py, (), None).unwrap().bind(py), "{}");

assert_repr(
obj.call1(py, ((('x', 1),),)).unwrap().as_ref(py),
"{'x': 1}",
);
assert_repr(obj.call1(py, ((('x', 1),),)).unwrap().bind(py), "{'x': 1}");
assert_repr(
obj.call_bound(py, (), Some(&[('x', 1)].into_py_dict_bound(py)))
.unwrap()
.as_ref(py),
.bind(py),
"{'x': 1}",
);
})
Expand All @@ -2037,7 +2022,7 @@ mod tests {
let obj: PyObject = PyDict::new_bound(py).into();
assert!(obj.call_method0(py, "asdf").is_err());
assert!(obj
.call_method(py, "nonexistent_method", (1,), None)
.call_method_bound(py, "nonexistent_method", (1,), None)
.is_err());
assert!(obj.call_method0(py, "nonexistent_method").is_err());
assert!(obj.call_method1(py, "nonexistent_method", (1,)).is_err());
Expand Down Expand Up @@ -2083,7 +2068,7 @@ a = A()

assert!(instance
.getattr(py, "foo")?
.as_ref(py)
.bind(py)
.eq(PyString::new_bound(py, "bar"))?);

instance.getattr(py, "foo")?;
Expand All @@ -2109,15 +2094,15 @@ a = A()

instance.getattr(py, foo).unwrap_err();
instance.setattr(py, foo, bar)?;
assert!(instance.getattr(py, foo)?.as_ref(py).eq(bar)?);
assert!(instance.getattr(py, foo)?.bind(py).eq(bar)?);
Ok(())
})
}

#[test]
fn invalid_attr() -> PyResult<()> {
Python::with_gil(|py| {
let instance: Py<PyAny> = py.eval("object()", None, None)?.into();
let instance: Py<PyAny> = py.eval_bound("object()", None, None)?.into();

instance.getattr(py, "foo").unwrap_err();

Expand All @@ -2130,7 +2115,7 @@ a = A()
#[test]
fn test_py2_from_py_object() {
Python::with_gil(|py| {
let instance: &PyAny = py.eval("object()", None, None).unwrap();
let instance = py.eval_bound("object()", None, None).unwrap();
let ptr = instance.as_ptr();
let instance: Bound<'_, PyAny> = instance.extract().unwrap();
assert_eq!(instance.as_ptr(), ptr);
Expand All @@ -2141,7 +2126,7 @@ a = A()
fn test_py2_into_py_object() {
Python::with_gil(|py| {
let instance = py
.eval("object()", None, None)
.eval_bound("object()", None, None)
.unwrap()
.as_borrowed()
.to_owned();
Expand Down
Loading
Loading