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 APIs using into_gil_ref (Part 2) #4166

Merged
merged 1 commit into from
May 9, 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
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
26 changes: 1 addition & 25 deletions pytests/src/pyclasses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,30 +63,6 @@ impl AssertingBaseClass {
}
}

#[allow(deprecated)]
mod deprecated {
use super::*;

#[pyclass(subclass)]
#[derive(Clone, Debug)]
pub struct AssertingBaseClassGilRef;

#[pymethods]
impl AssertingBaseClassGilRef {
#[new]
#[classmethod]
fn new(cls: &PyType, expected_type: &PyType) -> PyResult<Self> {
if !cls.is(expected_type) {
return Err(PyValueError::new_err(format!(
"{:?} != {:?}",
cls, expected_type
)));
}
Ok(Self)
}
}
}

#[pyclass]
struct ClassWithoutConstructor;

Expand All @@ -95,7 +71,7 @@ pub fn pyclasses(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_class::<EmptyClass>()?;
m.add_class::<PyClassIter>()?;
m.add_class::<AssertingBaseClass>()?;
m.add_class::<deprecated::AssertingBaseClassGilRef>()?;
m.add_class::<ClassWithoutConstructor>()?;

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
11 changes: 0 additions & 11 deletions pytests/tests/test_pyclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,6 @@ def test_new_classmethod():
_ = AssertingSubClass(expected_type=str)


def test_new_classmethod_gil_ref():
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:
def __new__(cls):
raise TypeError("No constructor defined")
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
12 changes: 5 additions & 7 deletions src/impl_/extract_argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -790,10 +790,8 @@ fn push_parameter_list(msg: &mut String, parameter_names: &[&str]) {

#[cfg(test)]
mod tests {
use crate::{
types::{IntoPyDict, PyTuple},
PyAny, Python,
};
use crate::types::{IntoPyDict, PyTuple};
use crate::Python;

use super::{push_parameter_list, FunctionDescription, NoVarargs, NoVarkeywords};

Expand All @@ -809,7 +807,7 @@ mod tests {
};

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

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

Python::with_gil(|py| {
let args = PyTuple::new_bound(py, Vec::<&PyAny>::new());
let args = PyTuple::empty_bound(py);
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
Loading
Loading