From 744a3f3d3f42ad1f5dfafee7702c36ed4da16ac7 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 3 Mar 2024 21:46:08 +0100 Subject: [PATCH] migration to pyo3 0.21 beta using the new Bound API This does still use GIL Refs in numpy's API but switches our internals to use the Bound API where appropriate. --- Cargo.toml | 4 ++-- examples/linalg/Cargo.toml | 2 +- examples/parallel/Cargo.toml | 2 +- examples/simple/Cargo.toml | 2 +- src/array.rs | 44 +++++++++++++++++++--------------- src/array_like.rs | 7 +++++- src/borrow/shared.rs | 22 ++++++++--------- src/dtype.rs | 46 +++++++++++++++++++----------------- src/npyffi/array.rs | 2 +- src/npyffi/mod.rs | 10 ++++---- src/npyffi/ufunc.rs | 2 +- src/strings.rs | 1 - src/sum_products.rs | 9 +++---- src/untyped_array.rs | 2 -- tests/to_py.rs | 1 + 15 files changed, 84 insertions(+), 72 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d0bc0b97d..99cfe4871 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,11 +22,11 @@ num-complex = ">= 0.2, < 0.5" num-integer = "0.1" num-traits = "0.2" ndarray = ">= 0.13, < 0.16" -pyo3 = { version = "0.20", default-features = false, features = ["macros"] } +pyo3 = { version = "0.21.0-beta", default-features = false, features = ["gil-refs", "macros"] } rustc-hash = "1.1" [dev-dependencies] -pyo3 = { version = "0.20", default-features = false, features = ["auto-initialize"] } +pyo3 = { version = "0.21.0-beta", default-features = false, features = ["auto-initialize", "gil-refs"] } nalgebra = { version = "0.32", default-features = false, features = ["std"] } [package.metadata.docs.rs] diff --git a/examples/linalg/Cargo.toml b/examples/linalg/Cargo.toml index 3ccb4172d..d2903414e 100644 --- a/examples/linalg/Cargo.toml +++ b/examples/linalg/Cargo.toml @@ -9,7 +9,7 @@ name = "rust_linalg" crate-type = ["cdylib"] [dependencies] -pyo3 = { version = "0.20", features = ["extension-module"] } +pyo3 = { version = "0.21.0-beta", features = ["extension-module"] } numpy = { path = "../.." } ndarray-linalg = { version = "0.14.1", features = ["openblas-system"] } diff --git a/examples/parallel/Cargo.toml b/examples/parallel/Cargo.toml index 8b0ef1a1e..cbbba4da1 100644 --- a/examples/parallel/Cargo.toml +++ b/examples/parallel/Cargo.toml @@ -9,7 +9,7 @@ name = "rust_parallel" crate-type = ["cdylib"] [dependencies] -pyo3 = { version = "0.20", features = ["extension-module", "multiple-pymethods"] } +pyo3 = { version = "0.21.0-beta", features = ["extension-module", "multiple-pymethods"] } numpy = { path = "../.." } ndarray = { version = "0.15", features = ["rayon", "blas"] } blas-src = { version = "0.8", features = ["openblas"] } diff --git a/examples/simple/Cargo.toml b/examples/simple/Cargo.toml index 763a6fd3a..147dd8f06 100644 --- a/examples/simple/Cargo.toml +++ b/examples/simple/Cargo.toml @@ -9,7 +9,7 @@ name = "rust_ext" crate-type = ["cdylib"] [dependencies] -pyo3 = { version = "0.20", features = ["extension-module", "abi3-py37"] } +pyo3 = { version = "0.21.0-beta", features = ["extension-module", "abi3-py37"] } numpy = { path = "../.." } [workspace] diff --git a/src/array.rs b/src/array.rs index 824b5b051..a57567996 100644 --- a/src/array.rs +++ b/src/array.rs @@ -17,9 +17,10 @@ use ndarray::{ }; use num_traits::AsPrimitive; use pyo3::{ - ffi, pyobject_native_type_base, types::PyModule, AsPyPointer, FromPyObject, IntoPy, Py, PyAny, - PyClassInitializer, PyDowncastError, PyErr, PyNativeType, PyObject, PyResult, PyTypeInfo, - Python, ToPyObject, + ffi, pyobject_native_type_base, + types::{DerefToPyAny, PyAnyMethods, PyModule}, + AsPyPointer, Bound, DowncastError, FromPyObject, IntoPy, Py, PyAny, PyErr, PyNativeType, + PyObject, PyResult, PyTypeInfo, Python, ToPyObject, }; use crate::borrow::{PyReadonlyArray, PyReadwriteArray}; @@ -118,13 +119,13 @@ pub type PyArray6 = PyArray; pub type PyArrayDyn = PyArray; /// Returns a handle to NumPy's multiarray module. -pub fn get_array_module<'py>(py: Python<'py>) -> PyResult<&PyModule> { - PyModule::import(py, npyffi::array::MOD_NAME) +pub fn get_array_module<'py>(py: Python<'py>) -> PyResult> { + PyModule::import_bound(py, npyffi::array::MOD_NAME) } -unsafe impl PyTypeInfo for PyArray { - type AsRefTarget = Self; +impl DerefToPyAny for PyArray {} +unsafe impl PyTypeInfo for PyArray { const NAME: &'static str = "PyArray"; const MODULE: Option<&'static str> = Some("numpy"); @@ -132,7 +133,7 @@ unsafe impl PyTypeInfo for PyArray { unsafe { npyffi::PY_ARRAY_API.get_type_object(py, npyffi::NpyTypes::PyArray_Type) } } - fn is_type_of(ob: &PyAny) -> bool { + fn is_type_of_bound(ob: &Bound<'_, PyAny>) -> bool { Self::extract::(ob).is_ok() } } @@ -189,8 +190,11 @@ impl IntoPy for PyArray { } impl<'py, T: Element, D: Dimension> FromPyObject<'py> for &'py PyArray { - fn extract(ob: &'py PyAny) -> PyResult { + fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult { + #[allow(clippy::map_clone)] // due to MSRV PyArray::extract(ob) + .map(Clone::clone) + .map(Bound::into_gil_ref) } } @@ -251,20 +255,22 @@ impl PyArray { } impl PyArray { - fn extract<'py, E>(ob: &'py PyAny) -> Result<&'py Self, E> + fn extract<'a, 'py, E>(ob: &'a Bound<'py, PyAny>) -> Result<&'a Bound<'py, Self>, E> where - E: From> + From + From>, + E: From> + From + From>, { // Check if the object is an array. let array = unsafe { if npyffi::PyArray_Check(ob.py(), ob.as_ptr()) == 0 { - return Err(PyDowncastError::new(ob, Self::NAME).into()); + return Err(DowncastError::new(ob, ::NAME).into()); } - &*(ob as *const PyAny as *const Self) + ob.downcast_unchecked() }; + let arr_gil_ref: &PyArray = array.as_gil_ref(); + // Check if the dimensionality matches `D`. - let src_ndim = array.ndim(); + let src_ndim = arr_gil_ref.ndim(); if let Some(dst_ndim) = D::NDIM { if src_ndim != dst_ndim { return Err(DimensionalityError::new(src_ndim, dst_ndim).into()); @@ -272,7 +278,7 @@ impl PyArray { } // Check if the element type matches `T`. - let src_dtype = array.dtype(); + let src_dtype = arr_gil_ref.dtype(); let dst_dtype = T::get_dtype(ob.py()); if !src_dtype.is_equiv_to(dst_dtype) { return Err(TypeError::new(src_dtype, dst_dtype).into()); @@ -399,11 +405,11 @@ impl PyArray { data_ptr: *const T, container: PySliceContainer, ) -> &'py Self { - let container = PyClassInitializer::from(container) - .create_cell(py) - .expect("Failed to create slice container"); + let container = Bound::new(py, container) + .expect("Failed to create slice container") + .into_ptr(); - Self::new_with_data(py, dims, strides, data_ptr, container as *mut PyAny) + Self::new_with_data(py, dims, strides, data_ptr, container.cast()) } /// Creates a NumPy array backed by `array` and ties its ownership to the Python object `container`. diff --git a/src/array_like.rs b/src/array_like.rs index 42695c286..a06122af3 100644 --- a/src/array_like.rs +++ b/src/array_like.rs @@ -2,7 +2,12 @@ use std::marker::PhantomData; use std::ops::Deref; use ndarray::{Array1, Dimension, Ix0, Ix1, Ix2, Ix3, Ix4, Ix5, Ix6, IxDyn}; -use pyo3::{intern, sync::GILOnceCell, types::PyDict, FromPyObject, Py, PyAny, PyResult}; +use pyo3::{ + intern, + sync::GILOnceCell, + types::{PyAnyMethods, PyDict}, + FromPyObject, Py, PyAny, PyResult, +}; use crate::sealed::Sealed; use crate::{get_array_module, Element, IntoPyArray, PyArray, PyReadonlyArray}; diff --git a/src/borrow/shared.rs b/src/borrow/shared.rs index 23590be2f..feccadc7c 100644 --- a/src/borrow/shared.rs +++ b/src/borrow/shared.rs @@ -5,10 +5,8 @@ use std::os::raw::{c_char, c_int}; use std::slice::from_raw_parts; use num_integer::gcd; -use pyo3::{ - exceptions::PyTypeError, once_cell::GILOnceCell, types::PyCapsule, Py, PyResult, PyTryInto, - Python, -}; +use pyo3::types::{PyAnyMethods, PyCapsuleMethods}; +use pyo3::{exceptions::PyTypeError, sync::GILOnceCell, types::PyCapsule, PyResult, Python}; use rustc_hash::FxHashMap; use crate::array::get_array_module; @@ -124,8 +122,8 @@ fn get_or_insert_shared<'py>(py: Python<'py>) -> PyResult<&'py Shared> { fn insert_shared<'py>(py: Python<'py>) -> PyResult<*const Shared> { let module = get_array_module(py)?; - let capsule: &PyCapsule = match module.getattr("_RUST_NUMPY_BORROW_CHECKING_API") { - Ok(capsule) => PyTryInto::try_into(capsule)?, + let capsule = match module.getattr("_RUST_NUMPY_BORROW_CHECKING_API") { + Ok(capsule) => capsule.downcast_into::()?, Err(_err) => { let flags: *mut BorrowFlags = Box::into_raw(Box::default()); @@ -138,7 +136,7 @@ fn insert_shared<'py>(py: Python<'py>) -> PyResult<*const Shared> { release_mut: release_mut_shared, }; - let capsule = PyCapsule::new_with_destructor( + let capsule = PyCapsule::new_bound_with_destructor( py, shared, Some(CString::new("_RUST_NUMPY_BORROW_CHECKING_API").unwrap()), @@ -147,13 +145,13 @@ fn insert_shared<'py>(py: Python<'py>) -> PyResult<*const Shared> { let _ = unsafe { Box::from_raw(shared.flags as *mut BorrowFlags) }; }, )?; - module.setattr("_RUST_NUMPY_BORROW_CHECKING_API", capsule)?; + module.setattr("_RUST_NUMPY_BORROW_CHECKING_API", &capsule)?; capsule } }; // SAFETY: All versions of the shared borrow checking API start with a version field. - let version = unsafe { *(capsule.pointer() as *mut u64) }; + let version = unsafe { *capsule.pointer().cast::() }; if version < 1 { return Err(PyTypeError::new_err(format!( "Version {} of borrow checking API is not supported by this version of rust-numpy", @@ -161,11 +159,13 @@ fn insert_shared<'py>(py: Python<'py>) -> PyResult<*const Shared> { ))); } + let ptr = capsule.pointer(); + // Intentionally leak a reference to the capsule // so we can safely cache a pointer into its interior. - forget(Py::::from(capsule)); + forget(capsule); - Ok(capsule.pointer() as *const Shared) + Ok(ptr.cast()) } // These entry points will be used to access the shared borrow checking API from this extension: diff --git a/src/dtype.rs b/src/dtype.rs index f8183c0ce..8ba095af8 100644 --- a/src/dtype.rs +++ b/src/dtype.rs @@ -11,9 +11,8 @@ use pyo3::{ exceptions::{PyIndexError, PyValueError}, ffi::{self, PyTuple_Size}, pyobject_native_type_extract, pyobject_native_type_named, - types::{PyDict, PyTuple, PyType}, - AsPyPointer, FromPyObject, FromPyPointer, PyAny, PyNativeType, PyObject, PyResult, PyTypeInfo, - Python, ToPyObject, + types::{PyAnyMethods, PyDict, PyDictMethods, PyTuple, PyType}, + AsPyPointer, Borrowed, PyAny, PyNativeType, PyObject, PyResult, PyTypeInfo, Python, ToPyObject, }; #[cfg(feature = "half")] use pyo3::{sync::GILOnceCell, IntoPy, Py}; @@ -53,8 +52,6 @@ pub struct PyArrayDescr(PyAny); pyobject_native_type_named!(PyArrayDescr); unsafe impl PyTypeInfo for PyArrayDescr { - type AsRefTarget = Self; - const NAME: &'static str = "PyArrayDescr"; const MODULE: Option<&'static str> = Some("numpy"); @@ -249,7 +246,9 @@ impl PyArrayDescr { if !self.has_subarray() { self } else { + #[allow(deprecated)] unsafe { + use pyo3::FromPyPointer; Self::from_borrowed_ptr(self.py(), (*(*self.as_dtype_ptr()).subarray).base as _) } } @@ -267,11 +266,9 @@ impl PyArrayDescr { Vec::new() } else { // NumPy guarantees that shape is a tuple of non-negative integers so this should never panic. - unsafe { - PyTuple::from_borrowed_ptr(self.py(), (*(*self.as_dtype_ptr()).subarray).shape) - } - .extract() - .unwrap() + unsafe { Borrowed::from_ptr(self.py(), (*(*self.as_dtype_ptr()).subarray).shape) } + .extract() + .unwrap() } } @@ -329,8 +326,8 @@ impl PyArrayDescr { if !self.has_fields() { return None; } - let names = unsafe { PyTuple::from_borrowed_ptr(self.py(), (*self.as_dtype_ptr()).names) }; - FromPyObject::extract(names).ok() + let names = unsafe { Borrowed::from_ptr(self.py(), (*self.as_dtype_ptr()).names) }; + names.extract().ok() } /// Returns the type descriptor and offset of the field with the given name. @@ -349,17 +346,22 @@ impl PyArrayDescr { "cannot get field information: type descriptor has no fields", )); } - let dict = unsafe { PyDict::from_borrowed_ptr(self.py(), (*self.as_dtype_ptr()).fields) }; + let dict = unsafe { Borrowed::from_ptr(self.py(), (*self.as_dtype_ptr()).fields) }; + let dict = unsafe { dict.downcast_unchecked::() }; // NumPy guarantees that fields are tuples of proper size and type, so this should never panic. let tuple = dict .get_item(name)? .ok_or_else(|| PyIndexError::new_err(name.to_owned()))? - .downcast::() + .downcast_into::() .unwrap(); // Note that we cannot just extract the entire tuple since the third element can be a title. - let dtype = FromPyObject::extract(tuple.as_ref().get_item(0).unwrap()).unwrap(); - let offset = FromPyObject::extract(tuple.as_ref().get_item(1).unwrap()).unwrap(); - Ok((dtype, offset)) + let dtype = tuple + .get_item(0) + .unwrap() + .downcast_into::() + .unwrap(); + let offset = tuple.get_item(1).unwrap().extract().unwrap(); + Ok((dtype.into_gil_ref(), offset)) } } @@ -548,8 +550,8 @@ mod tests { #[test] fn test_dtype_names() { - fn type_name<'py, T: Element>(py: Python<'py>) -> &str { - dtype::(py).typeobj().name().unwrap() + fn type_name<'py, T: Element>(py: Python<'py>) -> String { + dtype::(py).typeobj().qualname().unwrap() } Python::with_gil(|py| { assert_eq!(type_name::(py), "bool_"); @@ -589,7 +591,7 @@ mod tests { assert_eq!(dt.num(), NPY_TYPES::NPY_DOUBLE as c_int); assert_eq!(dt.flags(), 0); - assert_eq!(dt.typeobj().name().unwrap(), "float64"); + assert_eq!(dt.typeobj().qualname().unwrap(), "float64"); assert_eq!(dt.char(), b'd'); assert_eq!(dt.kind(), b'f'); assert_eq!(dt.byteorder(), b'='); @@ -625,7 +627,7 @@ mod tests { assert_eq!(dt.num(), NPY_TYPES::NPY_VOID as c_int); assert_eq!(dt.flags(), 0); - assert_eq!(dt.typeobj().name().unwrap(), "void"); + assert_eq!(dt.typeobj().qualname().unwrap(), "void"); assert_eq!(dt.char(), b'V'); assert_eq!(dt.kind(), b'V'); assert_eq!(dt.byteorder(), b'|'); @@ -663,7 +665,7 @@ mod tests { assert_ne!(dt.flags() & NPY_ITEM_HASOBJECT, 0); assert_ne!(dt.flags() & NPY_NEEDS_PYAPI, 0); assert_ne!(dt.flags() & NPY_ALIGNED_STRUCT, 0); - assert_eq!(dt.typeobj().name().unwrap(), "void"); + assert_eq!(dt.typeobj().qualname().unwrap(), "void"); assert_eq!(dt.char(), b'V'); assert_eq!(dt.kind(), b'V'); assert_eq!(dt.byteorder(), b'|'); diff --git a/src/npyffi/array.rs b/src/npyffi/array.rs index 60c373d22..f2ff651aa 100644 --- a/src/npyffi/array.rs +++ b/src/npyffi/array.rs @@ -9,7 +9,7 @@ use std::os::raw::*; use libc::FILE; use pyo3::{ ffi::{self, PyObject, PyTypeObject}, - once_cell::GILOnceCell, + sync::GILOnceCell, }; use crate::npyffi::*; diff --git a/src/npyffi/mod.rs b/src/npyffi/mod.rs index afc0b425e..1f71f2ca5 100644 --- a/src/npyffi/mod.rs +++ b/src/npyffi/mod.rs @@ -13,8 +13,8 @@ use std::mem::forget; use std::os::raw::c_void; use pyo3::{ - types::{PyCapsule, PyModule}, - Py, PyResult, PyTryInto, Python, + types::{PyAnyMethods, PyCapsule, PyCapsuleMethods, PyModule}, + PyResult, Python, }; fn get_numpy_api<'py>( @@ -22,14 +22,14 @@ fn get_numpy_api<'py>( module: &str, capsule: &str, ) -> PyResult<*const *const c_void> { - let module = PyModule::import(py, module)?; - let capsule: &PyCapsule = PyTryInto::try_into(module.getattr(capsule)?)?; + let module = PyModule::import_bound(py, module)?; + let capsule = module.getattr(capsule)?.downcast_into::()?; let api = capsule.pointer() as *const *const c_void; // Intentionally leak a reference to the capsule // so we can safely cache a pointer into its interior. - forget(Py::::from(capsule)); + forget(capsule); Ok(api) } diff --git a/src/npyffi/ufunc.rs b/src/npyffi/ufunc.rs index c906962ca..9f90e73f3 100644 --- a/src/npyffi/ufunc.rs +++ b/src/npyffi/ufunc.rs @@ -2,7 +2,7 @@ use std::os::raw::*; -use pyo3::{ffi::PyObject, once_cell::GILOnceCell}; +use pyo3::{ffi::PyObject, sync::GILOnceCell}; use crate::npyffi::*; diff --git a/src/strings.rs b/src/strings.rs index f42e31e47..cd1316b80 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -5,7 +5,6 @@ use std::cell::RefCell; use std::collections::hash_map::Entry; -use std::convert::TryInto; use std::fmt; use std::mem::size_of; use std::os::raw::c_char; diff --git a/src/sum_products.rs b/src/sum_products.rs index e99225cf4..360ea8b47 100644 --- a/src/sum_products.rs +++ b/src/sum_products.rs @@ -3,7 +3,8 @@ use std::ffi::{CStr, CString}; use std::ptr::null_mut; use ndarray::{Dimension, IxDyn}; -use pyo3::{AsPyPointer, FromPyObject, FromPyPointer, PyAny, PyNativeType, PyResult}; +use pyo3::types::PyAnyMethods; +use pyo3::{AsPyPointer, Bound, FromPyObject, PyNativeType, PyResult}; use crate::array::PyArray; use crate::dtype::Element; @@ -67,7 +68,7 @@ where let py = array1.py(); let obj = unsafe { let result = PY_ARRAY_API.PyArray_InnerProduct(py, array1.as_ptr(), array2.as_ptr()); - PyAny::from_owned_ptr_or_err(py, result)? + Bound::from_owned_ptr_or_err(py, result)? }; obj.extract() } @@ -125,7 +126,7 @@ where let py = array1.py(); let obj = unsafe { let result = PY_ARRAY_API.PyArray_MatrixProduct(py, array1.as_ptr(), array2.as_ptr()); - PyAny::from_owned_ptr_or_err(py, result)? + Bound::from_owned_ptr_or_err(py, result)? }; obj.extract() } @@ -155,7 +156,7 @@ where NPY_CASTING::NPY_NO_CASTING, null_mut(), ); - PyAny::from_owned_ptr_or_err(py, result)? + Bound::from_owned_ptr_or_err(py, result)? }; obj.extract() } diff --git a/src/untyped_array.rs b/src/untyped_array.rs index cd873a2b9..089afdd10 100644 --- a/src/untyped_array.rs +++ b/src/untyped_array.rs @@ -61,8 +61,6 @@ use crate::npyffi; pub struct PyUntypedArray(PyAny); unsafe impl PyTypeInfo for PyUntypedArray { - type AsRefTarget = Self; - const NAME: &'static str = "PyUntypedArray"; const MODULE: Option<&'static str> = Some("numpy"); diff --git a/tests/to_py.rs b/tests/to_py.rs index 0b60e0dbc..92c18626d 100644 --- a/tests/to_py.rs +++ b/tests/to_py.rs @@ -234,6 +234,7 @@ fn to_pyarray_object_vec() { Python::with_gil(|py| { let dict = PyDict::new(py); let string = PyString::new(py, "Hello:)"); + #[allow(clippy::useless_vec)] // otherwise we do not test the right trait impl let vec = vec![dict.to_object(py), string.to_object(py)]; let arr = vec.to_pyarray(py);