Skip to content

Commit

Permalink
feature gate APIs using into_gil_ref (Part 2)
Browse files Browse the repository at this point in the history
  • Loading branch information
Icxolu committed May 9, 2024
1 parent 2d19b7e commit 4f65a5c
Show file tree
Hide file tree
Showing 34 changed files with 107 additions and 133 deletions.
6 changes: 3 additions & 3 deletions guide/src/conversions/traits.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ use pyo3::prelude::*;

#[derive(FromPyObject)]
# #[derive(Debug)]
enum RustyEnum<'a> {
enum RustyEnum<'py> {
Int(usize), // input is a positive int
String(String), // input is a string
IntTuple(usize, usize), // input is a 2-tuple with positive ints
Expand All @@ -284,7 +284,7 @@ enum RustyEnum<'a> {
b: usize,
},
#[pyo3(transparent)]
CatchAll(&'a PyAny), // This extraction never fails
CatchAll(Bound<'py, PyAny>), // This extraction never fails
}
#
# use pyo3::types::{PyBytes, PyString};
Expand Down Expand Up @@ -394,7 +394,7 @@ enum RustyEnum<'a> {
# assert_eq!(
# b"text",
# match rust_thing {
# RustyEnum::CatchAll(i) => i.downcast::<PyBytes>()?.as_bytes(),
# RustyEnum::CatchAll(ref i) => i.downcast::<PyBytes>()?.as_bytes(),
# other => unreachable!("Error extracting: {:?}", other),
# }
# );
Expand Down
4 changes: 2 additions & 2 deletions guide/src/exception.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,5 @@ defines exceptions for several standard library modules.
[`PyErr`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html
[`PyResult`]: {{#PYO3_DOCS_URL}}/pyo3/type.PyResult.html
[`PyErr::from_value`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html#method.from_value
[`PyAny::is_instance`]: {{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.is_instance
[`PyAny::is_instance_of`]: {{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.is_instance_of
[`PyAny::is_instance`]: {{#PYO3_DOCS_URL}}/pyo3/types/trait.PyAnyMethods.html#tymethod.is_instance
[`PyAny::is_instance_of`]: {{#PYO3_DOCS_URL}}/pyo3/types/trait.PyAnyMethods.html#tymethod.is_instance_of
8 changes: 8 additions & 0 deletions guide/src/memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ held. (If PyO3 could not assume this, every PyO3 API would need to take a
very simple and easy-to-understand programs like this:

```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
Expand All @@ -57,9 +59,11 @@ it owns are decreased, releasing them to the Python garbage collector. Most
of the time we don't have to think about this, but consider the following:

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
# #[cfg(feature = "gil-refs")]
Python::with_gil(|py| -> PyResult<()> {
for _ in 0..10 {
#[allow(deprecated)] // py.eval() is part of the GIL Refs API
Expand Down Expand Up @@ -96,9 +100,11 @@ In general we don't want unbounded memory growth during loops! One workaround
is to acquire and release the GIL with each iteration of the loop.

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
# #[cfg(feature = "gil-refs")]
for _ in 0..10 {
Python::with_gil(|py| -> PyResult<()> {
#[allow(deprecated)] // py.eval() is part of the GIL Refs API
Expand All @@ -118,9 +124,11 @@ times. Another workaround is to work with the `GILPool` object directly, but
this is unsafe.

```rust
# #![allow(unused_imports)]
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
# #[cfg(feature = "gil-refs")]
Python::with_gil(|py| -> PyResult<()> {
for _ in 0..10 {
#[allow(deprecated)] // `new_pool` is not needed in code not using the GIL Refs API
Expand Down
2 changes: 1 addition & 1 deletion guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pyo3 = { version = "0.21", features = ["gil-refs"] }

The `PyTryFrom` trait has aged poorly, its `try_from` method now conflicts with `TryFrom::try_from` in the 2021 edition prelude. A lot of its functionality was also duplicated with `PyTypeInfo`.

To tighten up the PyO3 traits as part of the deprecation of the GIL Refs API the `PyTypeInfo` trait has had a simpler companion `PyTypeCheck`. The methods [`PyAny::downcast`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.downcast) and [`PyAny::downcast_exact`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.downcast_exact) no longer use `PyTryFrom` as a bound, instead using `PyTypeCheck` and `PyTypeInfo` respectively.
To tighten up the PyO3 traits as part of the deprecation of the GIL Refs API the `PyTypeInfo` trait has had a simpler companion `PyTypeCheck`. The methods `PyAny::downcast` and `PyAny::downcast_exact` no longer use `PyTryFrom` as a bound, instead using `PyTypeCheck` and `PyTypeInfo` respectively.

To migrate, switch all type casts to use `obj.downcast()` instead of `try_from(obj)` (and similar for `downcast_exact`).

Expand Down
2 changes: 2 additions & 0 deletions guide/src/types.md
Original file line number Diff line number Diff line change
Expand Up @@ -467,8 +467,10 @@ let _: &mut MyClass = &mut *py_ref_mut;
`PyCell<T>` was also accessed like a Python-native type.

```rust
#![allow(unused_imports)]
# use pyo3::prelude::*;
# #[pyclass] struct MyClass { }
# #[cfg(feature = "gil-refs")]
# Python::with_gil(|py| -> PyResult<()> {
#[allow(deprecated)] // &PyCell is part of the deprecate GIL Refs API
let cell: &PyCell<MyClass> = PyCell::new(py, MyClass {})?;
Expand Down
7 changes: 7 additions & 0 deletions pytests/src/pyclasses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl AssertingBaseClass {
}

#[allow(deprecated)]
#[cfg(feature = "gil-refs")]
mod deprecated {
use super::*;

Expand Down Expand Up @@ -95,7 +96,13 @@ pub fn pyclasses(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_class::<EmptyClass>()?;
m.add_class::<PyClassIter>()?;
m.add_class::<AssertingBaseClass>()?;
#[cfg(feature = "gil-refs")]
m.add_class::<deprecated::AssertingBaseClassGilRef>()?;
m.add_class::<ClassWithoutConstructor>()?;

#[pyfn(m)]
fn has_gil_refs() -> bool {
cfg!(feature = "gil-refs")
}
Ok(())
}
2 changes: 1 addition & 1 deletion pytests/src/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn array_to_array_i32(arr: [i32; 3]) -> [i32; 3] {
}

#[pyfunction]
fn vec_to_vec_pystring(vec: Vec<&PyString>) -> Vec<&PyString> {
fn vec_to_vec_pystring(vec: Vec<Bound<'_, PyString>>) -> Vec<Bound<'_, PyString>> {
vec
}

Expand Down
16 changes: 9 additions & 7 deletions pytests/tests/test_pyclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,16 @@ def test_new_classmethod():


def test_new_classmethod_gil_ref():
class AssertingSubClass(pyclasses.AssertingBaseClassGilRef):
pass
if pyclasses.has_gil_refs():

# The `AssertingBaseClass` constructor errors if it is not passed the
# relevant subclass.
_ = AssertingSubClass(expected_type=AssertingSubClass)
with pytest.raises(ValueError):
_ = AssertingSubClass(expected_type=str)
class AssertingSubClass(pyclasses.AssertingBaseClassGilRef):
pass

# The `AssertingBaseClass` constructor errors if it is not passed the
# relevant subclass.
_ = AssertingSubClass(expected_type=AssertingSubClass)
with pytest.raises(ValueError):
_ = AssertingSubClass(expected_type=str)


class ClassWithoutConstructorPy:
Expand Down
1 change: 1 addition & 0 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ where
}

#[allow(deprecated)]
#[cfg(feature = "gil-refs")]
impl<'py, T> FromPyObject<'py> for &'py crate::PyCell<T>
where
T: PyClass,
Expand Down
2 changes: 1 addition & 1 deletion src/conversions/chrono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ impl FromPyObject<'_> for FixedOffset {
/// does not supports microseconds.
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<FixedOffset> {
#[cfg(not(Py_LIMITED_API))]
let ob: &PyTzInfo = ob.extract()?;
let ob = ob.downcast::<PyTzInfo>()?;
#[cfg(Py_LIMITED_API)]
check_type(ob, &DatetimeTypes::get(ob.py()).tzinfo, "PyTzInfo")?;

Expand Down
5 changes: 3 additions & 2 deletions src/conversions/std/osstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ impl<'a> IntoPy<PyObject> for &'a OsString {

#[cfg(test)]
mod tests {
use crate::types::{PyAnyMethods, PyStringMethods};
use crate::{types::PyString, IntoPy, PyObject, Python, ToPyObject};
use std::fmt::Debug;
use std::{
Expand Down Expand Up @@ -179,7 +180,7 @@ mod tests {
Python::with_gil(|py| {
fn test_roundtrip<T: ToPyObject + AsRef<OsStr> + Debug>(py: Python<'_>, obj: T) {
let pyobject = obj.to_object(py);
let pystring: &PyString = pyobject.extract(py).unwrap();
let pystring = pyobject.downcast_bound::<PyString>(py).unwrap();
assert_eq!(pystring.to_string_lossy(), obj.as_ref().to_string_lossy());
let roundtripped_obj: OsString = pystring.extract().unwrap();
assert_eq!(obj.as_ref(), roundtripped_obj.as_os_str());
Expand All @@ -200,7 +201,7 @@ mod tests {
obj: T,
) {
let pyobject = obj.clone().into_py(py);
let pystring: &PyString = pyobject.extract(py).unwrap();
let pystring = pyobject.downcast_bound::<PyString>(py).unwrap();
assert_eq!(pystring.to_string_lossy(), obj.as_ref().to_string_lossy());
let roundtripped_obj: OsString = pystring.extract().unwrap();
assert!(obj.as_ref() == roundtripped_obj.as_os_str());
Expand Down
5 changes: 3 additions & 2 deletions src/conversions/std/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl<'a> IntoPy<PyObject> for &'a PathBuf {

#[cfg(test)]
mod tests {
use crate::types::{PyAnyMethods, PyStringMethods};
use crate::{types::PyString, IntoPy, PyObject, Python, ToPyObject};
use std::borrow::Cow;
use std::fmt::Debug;
Expand Down Expand Up @@ -95,7 +96,7 @@ mod tests {
Python::with_gil(|py| {
fn test_roundtrip<T: ToPyObject + AsRef<Path> + Debug>(py: Python<'_>, obj: T) {
let pyobject = obj.to_object(py);
let pystring: &PyString = pyobject.extract(py).unwrap();
let pystring = pyobject.downcast_bound::<PyString>(py).unwrap();
assert_eq!(pystring.to_string_lossy(), obj.as_ref().to_string_lossy());
let roundtripped_obj: PathBuf = pystring.extract().unwrap();
assert_eq!(obj.as_ref(), roundtripped_obj.as_path());
Expand All @@ -116,7 +117,7 @@ mod tests {
obj: T,
) {
let pyobject = obj.clone().into_py(py);
let pystring: &PyString = pyobject.extract(py).unwrap();
let pystring = pyobject.downcast_bound::<PyString>(py).unwrap();
assert_eq!(pystring.to_string_lossy(), obj.as_ref().to_string_lossy());
let roundtripped_obj: PathBuf = pystring.extract().unwrap();
assert_eq!(obj.as_ref(), roundtripped_obj.as_path());
Expand Down
2 changes: 1 addition & 1 deletion src/derive_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ impl<'a> PyFunctionArguments<'a> {
match self {
PyFunctionArguments::Python(py) => (py, None),
PyFunctionArguments::PyModule(module) => {
let py = module.py();
let py = crate::PyNativeType::py(module);
(py, Some(module))
}
}
Expand Down
1 change: 1 addition & 0 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@ where
}

/// Convert `PyDowncastError` to Python `TypeError`.
#[cfg(feature = "gil-refs")]
impl<'a> std::convert::From<PyDowncastError<'a>> for PyErr {
fn from(err: PyDowncastError<'_>) -> PyErr {
let args = PyDowncastErrorArguments {
Expand Down
6 changes: 5 additions & 1 deletion src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ macro_rules! impl_exception_boilerplate {

$crate::impl_exception_boilerplate_bound!($name);

#[cfg(feature = "gil-refs")]
impl ::std::error::Error for $name {
fn source(&self) -> ::std::option::Option<&(dyn ::std::error::Error + 'static)> {
unsafe {
Expand All @@ -58,6 +59,7 @@ macro_rules! impl_exception_boilerplate_bound {
///
/// [`PyErr`]: https://docs.rs/pyo3/latest/pyo3/struct.PyErr.html "PyErr in pyo3"
#[inline]
#[allow(dead_code)]
pub fn new_err<A>(args: A) -> $crate::PyErr
where
A: $crate::PyErrArguments + ::std::marker::Send + ::std::marker::Sync + 'static,
Expand Down Expand Up @@ -881,7 +883,9 @@ mod tests {
use super::*;
use crate::types::any::PyAnyMethods;
use crate::types::{IntoPyDict, PyDict};
use crate::{PyErr, PyNativeType};
use crate::PyErr;
#[cfg(feature = "gil-refs")]
use crate::PyNativeType;

import_exception_bound!(socket, gaierror);
import_exception_bound!(email.errors, MessageError);
Expand Down
32 changes: 10 additions & 22 deletions src/impl_/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,39 +29,27 @@ impl<T> GilRefs<T> {
}

impl GilRefs<Python<'_>> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(since = "0.21.0", note = "use `wrap_pyfunction_bound!` instead")
)]
#[deprecated(since = "0.21.0", note = "use `wrap_pyfunction_bound!` instead")]
pub fn is_python(&self) {}
}

impl<T: IsGilRef> GilRefs<T> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `&Bound<'_, T>` instead for this function argument"
)
#[deprecated(
since = "0.21.0",
note = "use `&Bound<'_, T>` instead for this function argument"
)]
pub fn function_arg(&self) {}
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor"
)
#[deprecated(
since = "0.21.0",
note = "use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor"
)]
pub fn from_py_with_arg(&self) {}
}

impl<T: IsGilRef> OptionGilRefs<Option<T>> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Option<&Bound<'_, T>>` instead for this function argument"
)
#[deprecated(
since = "0.21.0",
note = "use `Option<&Bound<'_, T>>` instead for this function argument"
)]
pub fn function_arg(&self) {}
}
Expand Down
8 changes: 4 additions & 4 deletions src/impl_/extract_argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ fn push_parameter_list(msg: &mut String, parameter_names: &[&str]) {
mod tests {
use crate::{
types::{IntoPyDict, PyTuple},
PyAny, Python,
Bound, PyAny, Python,
};

use super::{push_parameter_list, FunctionDescription, NoVarargs, NoVarkeywords};
Expand All @@ -809,7 +809,7 @@ mod tests {
};

Python::with_gil(|py| {
let args = PyTuple::new_bound(py, Vec::<&PyAny>::new());
let args = PyTuple::new_bound(py, Vec::<Bound<'_, PyAny>>::new());
let kwargs = [("foo", 0u8)].into_py_dict_bound(py);
let err = unsafe {
function_description
Expand Down Expand Up @@ -840,7 +840,7 @@ mod tests {
};

Python::with_gil(|py| {
let args = PyTuple::new_bound(py, Vec::<&PyAny>::new());
let args = PyTuple::new_bound(py, Vec::<Bound<'_, PyAny>>::new());
let kwargs = [(1u8, 1u8)].into_py_dict_bound(py);
let err = unsafe {
function_description
Expand Down Expand Up @@ -871,7 +871,7 @@ mod tests {
};

Python::with_gil(|py| {
let args = PyTuple::new_bound(py, Vec::<&PyAny>::new());
let args = PyTuple::new_bound(py, Vec::<Bound<'_, PyAny>>::new());
let mut output = [None, None];
let err = unsafe {
function_description.extract_arguments_tuple_dict::<NoVarargs, NoVarkeywords>(
Expand Down
4 changes: 2 additions & 2 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ impl<T> Py<T> {
}

/// Returns whether `self` and `other` point to the same object. To compare
/// the equality of two objects (the `==` operator), use [`eq`](PyAny::eq).
/// the equality of two objects (the `==` operator), use [`eq`](PyAnyMethods::eq).
///
/// This is equivalent to the Python expression `self is other`.
#[inline]
Expand Down Expand Up @@ -2142,7 +2142,7 @@ a = A()
fn test_is_ellipsis() {
Python::with_gil(|py| {
let v = py
.eval("...", None, None)
.eval_bound("...", None, None)
.map_err(|e| e.display(py))
.unwrap()
.to_object(py);
Expand Down
4 changes: 2 additions & 2 deletions src/pycell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
//! The [`#[pymethods]`](crate::pymethods) proc macro will generate this wrapper function (and more),
//! using [`PyCell`] under the hood:
//!
//! ```rust
//! ```rust,ignore
//! # use pyo3::prelude::*;
//! # #[pyclass]
//! # struct Number {
Expand Down Expand Up @@ -148,7 +148,7 @@
//! ```
//!
//! It is better to write that function like this:
//! ```rust
//! ```rust,ignore
//! # #![allow(deprecated)]
//! # use pyo3::prelude::*;
//! # #[pyclass]
Expand Down

0 comments on commit 4f65a5c

Please sign in to comment.