Skip to content

Commit

Permalink
remove gil-refs from wrap_pyfunction and from_py_with (#4343)
Browse files Browse the repository at this point in the history
* deprecate `wrap_pyfunction_bound`

* remove `from_py_with` gil-ref extractors

* reword deprecation msg

Co-authored-by: David Hewitt <[email protected]>

---------

Co-authored-by: David Hewitt <[email protected]>
  • Loading branch information
Icxolu and davidhewitt authored Jul 25, 2024
1 parent 9a8e5b4 commit cf55960
Show file tree
Hide file tree
Showing 38 changed files with 101 additions and 251 deletions.
2 changes: 1 addition & 1 deletion guide/src/function.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ The `#[pyo3]` attribute can be used on individual arguments to modify properties
}

# Python::with_gil(|py| {
# let f = pyo3::wrap_pyfunction_bound!(object_length)(py).unwrap();
# let f = pyo3::wrap_pyfunction!(object_length)(py).unwrap();
# assert_eq!(f.call1((vec![1, 2, 3],)).unwrap().extract::<usize>().unwrap(), 3);
# });
```
Expand Down
8 changes: 4 additions & 4 deletions guide/src/function/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn check_positive(x: i32) -> PyResult<()> {
#
# fn main(){
# Python::with_gil(|py|{
# let fun = pyo3::wrap_pyfunction_bound!(check_positive, py).unwrap();
# let fun = pyo3::wrap_pyfunction!(check_positive, py).unwrap();
# fun.call1((-1,)).unwrap_err();
# fun.call1((1,)).unwrap();
# });
Expand Down Expand Up @@ -72,7 +72,7 @@ fn parse_int(x: &str) -> Result<usize, ParseIntError> {

# fn main() {
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(parse_int, py).unwrap();
# let fun = pyo3::wrap_pyfunction!(parse_int, py).unwrap();
# let value: usize = fun.call1(("5",)).unwrap().extract().unwrap();
# assert_eq!(value, 5);
# });
Expand Down Expand Up @@ -132,7 +132,7 @@ fn connect(s: String) -> Result<(), CustomIOError> {

fn main() {
Python::with_gil(|py| {
let fun = pyo3::wrap_pyfunction_bound!(connect, py).unwrap();
let fun = pyo3::wrap_pyfunction!(connect, py).unwrap();
let err = fun.call1(("0.0.0.0",)).unwrap_err();
assert!(err.is_instance_of::<PyOSError>(py));
});
Expand Down Expand Up @@ -224,7 +224,7 @@ fn wrapped_get_x() -> Result<i32, MyOtherError> {

# fn main() {
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(wrapped_get_x, py).unwrap();
# let fun = pyo3::wrap_pyfunction!(wrapped_get_x, py).unwrap();
# let value: usize = fun.call0().unwrap().extract().unwrap();
# assert_eq!(value, 5);
# });
Expand Down
10 changes: 5 additions & 5 deletions guide/src/function/signature.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ fn increment(x: u64, amount: Option<u64>) -> u64 {
#
# fn main() -> PyResult<()> {
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(increment, py)?;
# let fun = pyo3::wrap_pyfunction!(increment, py)?;
#
# let inspect = PyModule::import_bound(py, "inspect")?.getattr("signature")?;
# let sig: String = inspect
Expand Down Expand Up @@ -176,7 +176,7 @@ fn increment(x: u64, amount: Option<u64>) -> u64 {
#
# fn main() -> PyResult<()> {
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(increment, py)?;
# let fun = pyo3::wrap_pyfunction!(increment, py)?;
#
# let inspect = PyModule::import_bound(py, "inspect")?.getattr("signature")?;
# let sig: String = inspect
Expand Down Expand Up @@ -216,7 +216,7 @@ fn add(a: u64, b: u64) -> u64 {
#
# fn main() -> PyResult<()> {
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(add, py)?;
# let fun = pyo3::wrap_pyfunction!(add, py)?;
#
# let doc: String = fun.getattr("__doc__")?.extract()?;
# assert_eq!(doc, "This function adds two unsigned 64-bit integers.");
Expand Down Expand Up @@ -264,7 +264,7 @@ fn add(a: u64, b: u64) -> u64 {
#
# fn main() -> PyResult<()> {
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(add, py)?;
# let fun = pyo3::wrap_pyfunction!(add, py)?;
#
# let doc: String = fun.getattr("__doc__")?.extract()?;
# assert_eq!(doc, "This function adds two unsigned 64-bit integers.");
Expand Down Expand Up @@ -306,7 +306,7 @@ fn add(a: u64, b: u64) -> u64 {
#
# fn main() -> PyResult<()> {
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(add, py)?;
# let fun = pyo3::wrap_pyfunction!(add, py)?;
#
# let doc: String = fun.getattr("__doc__")?.extract()?;
# assert_eq!(doc, "This function adds two unsigned 64-bit integers.");
Expand Down
4 changes: 2 additions & 2 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -884,9 +884,9 @@ fn function_with_defaults(a: i32, b: i32, c: i32) {}

# fn main() {
# Python::with_gil(|py| {
# let simple = wrap_pyfunction_bound!(simple_function, py).unwrap();
# let simple = wrap_pyfunction!(simple_function, py).unwrap();
# assert_eq!(simple.getattr("__text_signature__").unwrap().to_string(), "(a, b, c)");
# let defaulted = wrap_pyfunction_bound!(function_with_defaults, py).unwrap();
# let defaulted = wrap_pyfunction!(function_with_defaults, py).unwrap();
# assert_eq!(defaulted.getattr("__text_signature__").unwrap().to_string(), "(a, b=1, c=2)");
# })
# }
Expand Down
10 changes: 5 additions & 5 deletions pytests/src/enums.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use pyo3::{
pyclass, pyfunction, pymodule,
types::{PyModule, PyModuleMethods},
wrap_pyfunction_bound, Bound, PyResult,
wrap_pyfunction, Bound, PyResult,
};

#[pymodule]
Expand All @@ -11,10 +11,10 @@ pub fn enums(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_class::<SimpleTupleEnum>()?;
m.add_class::<TupleEnum>()?;
m.add_class::<MixedComplexEnum>()?;
m.add_wrapped(wrap_pyfunction_bound!(do_simple_stuff))?;
m.add_wrapped(wrap_pyfunction_bound!(do_complex_stuff))?;
m.add_wrapped(wrap_pyfunction_bound!(do_tuple_stuff))?;
m.add_wrapped(wrap_pyfunction_bound!(do_mixed_complex_stuff))?;
m.add_wrapped(wrap_pyfunction!(do_simple_stuff))?;
m.add_wrapped(wrap_pyfunction!(do_complex_stuff))?;
m.add_wrapped(wrap_pyfunction!(do_tuple_stuff))?;
m.add_wrapped(wrap_pyfunction!(do_mixed_complex_stuff))?;
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion src/conversions/anyhow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
//!
//! fn main() {
//! let error = Python::with_gil(|py| -> PyResult<Vec<u8>> {
//! let fun = wrap_pyfunction_bound!(py_open, py)?;
//! let fun = wrap_pyfunction!(py_open, py)?;
//! let text = fun.call1(("foo.txt",))?.extract::<Vec<u8>>()?;
//! Ok(text)
//! }).unwrap_err();
Expand Down
2 changes: 1 addition & 1 deletion src/conversions/eyre.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
//!
//! fn main() {
//! let error = Python::with_gil(|py| -> PyResult<Vec<u8>> {
//! let fun = wrap_pyfunction_bound!(py_open, py)?;
//! let fun = wrap_pyfunction!(py_open, py)?;
//! let text = fun.call1(("foo.txt",))?.extract::<Vec<u8>>()?;
//! Ok(text)
//! }).unwrap_err();
Expand Down
4 changes: 2 additions & 2 deletions src/coroutine/waker.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::sync::GILOnceCell;
use crate::types::any::PyAnyMethods;
use crate::types::PyCFunction;
use crate::{intern, wrap_pyfunction_bound, Bound, Py, PyAny, PyObject, PyResult, Python};
use crate::{intern, wrap_pyfunction, Bound, Py, PyAny, PyObject, PyResult, Python};
use pyo3_macros::pyfunction;
use std::sync::Arc;
use std::task::Wake;
Expand Down Expand Up @@ -71,7 +71,7 @@ impl LoopAndFuture {
fn set_result(&self, py: Python<'_>) -> PyResult<()> {
static RELEASE_WAITER: GILOnceCell<Py<PyCFunction>> = GILOnceCell::new();
let release_waiter = RELEASE_WAITER.get_or_try_init(py, || {
wrap_pyfunction_bound!(release_waiter, py).map(Bound::unbind)
wrap_pyfunction!(release_waiter, py).map(Bound::unbind)
})?;
// `Future.set_result` must be called in event loop thread,
// so it requires `call_soon_threadsafe`
Expand Down
33 changes: 0 additions & 33 deletions src/derive_utils.rs

This file was deleted.

4 changes: 2 additions & 2 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl PyErr {
/// }
/// #
/// # Python::with_gil(|py| {
/// # let fun = pyo3::wrap_pyfunction_bound!(always_throws, py).unwrap();
/// # let fun = pyo3::wrap_pyfunction!(always_throws, py).unwrap();
/// # let err = fun.call0().expect_err("called a function that should always return an error but the return value was Ok");
/// # assert!(err.is_instance_of::<PyTypeError>(py))
/// # });
Expand All @@ -145,7 +145,7 @@ impl PyErr {
/// }
/// #
/// # Python::with_gil(|py| {
/// # let fun = pyo3::wrap_pyfunction_bound!(always_throws, py).unwrap();
/// # let fun = pyo3::wrap_pyfunction!(always_throws, py).unwrap();
/// # let err = fun.call0().expect_err("called a function that should always return an error but the return value was Ok");
/// # assert!(err.is_instance_of::<PyTypeError>(py))
/// # });
Expand Down
4 changes: 2 additions & 2 deletions src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ macro_rules! import_exception_bound {
/// }
/// # fn main() -> PyResult<()> {
/// # Python::with_gil(|py| -> PyResult<()> {
/// # let fun = wrap_pyfunction_bound!(raise_myerror, py)?;
/// # let fun = wrap_pyfunction!(raise_myerror, py)?;
/// # let locals = pyo3::types::PyDict::new_bound(py);
/// # locals.set_item("MyError", py.get_type_bound::<MyError>())?;
/// # locals.set_item("raise_myerror", fun)?;
Expand Down Expand Up @@ -332,7 +332,7 @@ fn always_throws() -> PyResult<()> {
}
#
# Python::with_gil(|py| {
# let fun = pyo3::wrap_pyfunction_bound!(always_throws, py).unwrap();
# let fun = pyo3::wrap_pyfunction!(always_throws, py).unwrap();
# let err = fun.call0().expect_err(\"called a function that should always return an error but the return value was Ok\");
# assert!(err.is_instance_of::<Py", $name, ">(py))
# });
Expand Down
6 changes: 3 additions & 3 deletions src/impl_/extract_argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ where
pub fn from_py_with<'a, 'py, T>(
obj: &'a Bound<'py, PyAny>,
arg_name: &str,
extractor: impl Into<super::frompyobject::Extractor<'a, 'py, T>>,
extractor: fn(&'a Bound<'py, PyAny>) -> PyResult<T>,
) -> PyResult<T> {
match extractor.into().call(obj) {
match extractor(obj) {
Ok(value) => Ok(value),
Err(e) => Err(argument_extraction_error(obj.py(), arg_name, e)),
}
Expand All @@ -184,7 +184,7 @@ pub fn from_py_with<'a, 'py, T>(
pub fn from_py_with_with_default<'a, 'py, T>(
obj: Option<&'a Bound<'py, PyAny>>,
arg_name: &str,
extractor: impl Into<super::frompyobject::Extractor<'a, 'py, T>>,
extractor: fn(&'a Bound<'py, PyAny>) -> PyResult<T>,
default: fn() -> T,
) -> PyResult<T> {
match obj {
Expand Down
37 changes: 4 additions & 33 deletions src/impl_/frompyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,6 @@ use crate::types::any::PyAnyMethods;
use crate::Bound;
use crate::{exceptions::PyTypeError, FromPyObject, PyAny, PyErr, PyResult, Python};

pub enum Extractor<'a, 'py, T> {
Bound(fn(&'a Bound<'py, PyAny>) -> PyResult<T>),
#[cfg(feature = "gil-refs")]
GilRef(fn(&'a PyAny) -> PyResult<T>),
}

impl<'a, 'py, T> From<fn(&'a Bound<'py, PyAny>) -> PyResult<T>> for Extractor<'a, 'py, T> {
fn from(value: fn(&'a Bound<'py, PyAny>) -> PyResult<T>) -> Self {
Self::Bound(value)
}
}

#[cfg(feature = "gil-refs")]
impl<'a, T> From<fn(&'a PyAny) -> PyResult<T>> for Extractor<'a, '_, T> {
fn from(value: fn(&'a PyAny) -> PyResult<T>) -> Self {
Self::GilRef(value)
}
}

impl<'a, 'py, T> Extractor<'a, 'py, T> {
pub(crate) fn call(self, obj: &'a Bound<'py, PyAny>) -> PyResult<T> {
match self {
Extractor::Bound(f) => f(obj),
#[cfg(feature = "gil-refs")]
Extractor::GilRef(f) => f(obj.as_gil_ref()),
}
}
}

#[cold]
pub fn failed_to_extract_enum(
py: Python<'_>,
Expand Down Expand Up @@ -91,12 +62,12 @@ where
}

pub fn extract_struct_field_with<'a, 'py, T>(
extractor: impl Into<Extractor<'a, 'py, T>>,
extractor: fn(&'a Bound<'py, PyAny>) -> PyResult<T>,
obj: &'a Bound<'py, PyAny>,
struct_name: &str,
field_name: &str,
) -> PyResult<T> {
match extractor.into().call(obj) {
match extractor(obj) {
Ok(value) => Ok(value),
Err(err) => Err(failed_to_extract_struct_field(
obj.py(),
Expand Down Expand Up @@ -142,12 +113,12 @@ where
}

pub fn extract_tuple_struct_field_with<'a, 'py, T>(
extractor: impl Into<Extractor<'a, 'py, T>>,
extractor: fn(&'a Bound<'py, PyAny>) -> PyResult<T>,
obj: &'a Bound<'py, PyAny>,
struct_name: &str,
index: usize,
) -> PyResult<T> {
match extractor.into().call(obj) {
match extractor(obj) {
Ok(value) => Ok(value),
Err(err) => Err(failed_to_extract_tuple_struct_field(
obj.py(),
Expand Down
38 changes: 0 additions & 38 deletions src/impl_/pyfunction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,46 +35,8 @@ impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for &'_ Borrowed<'_, '
}
}

// For Python<'py>, only the GIL Ref form exists to avoid causing type inference to kick in.
// The `wrap_pyfunction_bound!` macro is needed for the Bound form.
#[cfg(feature = "gil-refs")]
impl<'py> WrapPyFunctionArg<'py, &'py PyCFunction> for Python<'py> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<&'py PyCFunction> {
PyCFunction::internal_new(self, method_def, None).map(Bound::into_gil_ref)
}
}

#[cfg(not(feature = "gil-refs"))]
impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for Python<'py> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
PyCFunction::internal_new(self, method_def, None)
}
}

#[cfg(feature = "gil-refs")]
impl<'py> WrapPyFunctionArg<'py, &'py PyCFunction> for &'py PyModule {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<&'py PyCFunction> {
use crate::PyNativeType;
PyCFunction::internal_new(self.py(), method_def, Some(&self.as_borrowed()))
.map(Bound::into_gil_ref)
}
}

/// Helper for `wrap_pyfunction_bound!` to guarantee return type of `Bound<'py, PyCFunction>`.
pub struct OnlyBound<T>(pub T);

impl<'py, T> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for OnlyBound<T>
where
T: WrapPyFunctionArg<'py, Bound<'py, PyCFunction>>,
{
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
WrapPyFunctionArg::wrap_pyfunction(self.0, method_def)
}
}

#[cfg(feature = "gil-refs")]
impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for OnlyBound<Python<'py>> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
PyCFunction::internal_new(self.0, method_def, None)
}
}
4 changes: 0 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,6 @@ pub mod conversion;
mod conversions;
#[cfg(feature = "experimental-async")]
pub mod coroutine;
#[macro_use]
#[doc(hidden)]
#[cfg(feature = "gil-refs")]
pub mod derive_utils;
mod err;
pub mod exceptions;
pub mod ffi;
Expand Down
Loading

0 comments on commit cf55960

Please sign in to comment.