Skip to content

Commit

Permalink
Removed PyClone trait, moved it methods to Element, and updated i…
Browse files Browse the repository at this point in the history
…mplementations/usages.

The updates including renaming the `py_clone` method to `clone_ref` to match that of `Py::clone_ref`, and replacing the `impl_py_clone!` macro with `clone_methods_impl!`.
  • Loading branch information
JRRudy1 authored and davidhewitt committed Oct 6, 2024
1 parent ae95f69 commit 71a30c2
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 143 deletions.
4 changes: 2 additions & 2 deletions src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1473,7 +1473,7 @@ unsafe fn clone_elements<T: Element>(py: Python<'_>, elems: &[T], data_ptr: &mut
*data_ptr = data_ptr.add(elems.len());
} else {
for elem in elems {
data_ptr.write(elem.py_clone(py));
data_ptr.write(elem.clone_ref(py));
*data_ptr = data_ptr.add(1);
}
}
Expand Down Expand Up @@ -2205,7 +2205,7 @@ impl<'py, T, D> PyArrayMethods<'py, T, D> for Bound<'py, PyArray<T, D>> {
Idx: NpyIndex<Dim = D>,
{
let element = unsafe { self.get(index) };
element.map(|elem| elem.py_clone(self.py()))
element.map(|elem| elem.clone_ref(self.py()))
}

fn to_dyn(&self) -> &Bound<'py, PyArray<T, IxDyn>> {
Expand Down
4 changes: 2 additions & 2 deletions src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ where
let array = PyArray::<A, _>::new_bound(py, dim, false);
let mut data_ptr = array.data();
for item in self.iter() {
data_ptr.write(item.py_clone(py));
data_ptr.write(item.clone_ref(py));
data_ptr = data_ptr.add(1);
}
array
Expand Down Expand Up @@ -236,7 +236,7 @@ where
ptr::copy_nonoverlapping(self.data.ptr(), data_ptr, self.len());
} else {
for item in self.iter() {
data_ptr.write(item.py_clone(py));
data_ptr.write(item.clone_ref(py));
data_ptr = data_ptr.add(1);
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use std::marker::PhantomData;
use pyo3::{sync::GILProtected, Bound, Py, Python};
use rustc_hash::FxHashMap;

use crate::dtype::{Element, PyArrayDescr, PyArrayDescrMethods, impl_py_clone};
use crate::dtype::{Element, PyArrayDescr, PyArrayDescrMethods, clone_methods_impl};
use crate::npyffi::{
PyArray_DatetimeDTypeMetaData, PyDataType_C_METADATA, NPY_DATETIMEUNIT, NPY_TYPES,
};
Expand Down Expand Up @@ -155,8 +155,6 @@ impl<U: Unit> From<Datetime<U>> for i64 {
}
}

impl_py_clone!(Datetime<U>; [U: Unit]);

unsafe impl<U: Unit> Element for Datetime<U> {
const IS_COPY: bool = true;

Expand All @@ -165,6 +163,8 @@ unsafe impl<U: Unit> Element for Datetime<U> {

DTYPES.from_unit(py, U::UNIT)
}

clone_methods_impl!(Self);
}

impl<U: Unit> fmt::Debug for Datetime<U> {
Expand Down Expand Up @@ -192,8 +192,6 @@ impl<U: Unit> From<Timedelta<U>> for i64 {
}
}

impl_py_clone!(Timedelta<U>; [U: Unit]);

unsafe impl<U: Unit> Element for Timedelta<U> {
const IS_COPY: bool = true;

Expand All @@ -202,6 +200,8 @@ unsafe impl<U: Unit> Element for Timedelta<U> {

DTYPES.from_unit(py, U::UNIT)
}

clone_methods_impl!(Self);
}

impl<U: Unit> fmt::Debug for Timedelta<U> {
Expand Down
148 changes: 61 additions & 87 deletions src/dtype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ use pyo3::{
ffi::{self, PyTuple_Size},
pyobject_native_type_extract, pyobject_native_type_named,
types::{PyAnyMethods, PyDict, PyDictMethods, PyTuple, PyType},
Borrowed, Bound, PyAny, PyObject, PyResult, PyTypeInfo, Python, ToPyObject,
Borrowed, Bound, Py, PyAny, PyObject, PyResult, PyTypeInfo, Python, ToPyObject,
};
#[cfg(feature = "half")]
use pyo3::{sync::GILOnceCell, Py};
use pyo3::{sync::GILOnceCell};
#[cfg(feature = "gil-refs")]
use pyo3::{AsPyPointer, PyNativeType};

Expand Down Expand Up @@ -644,56 +644,6 @@ impl<'py> PyArrayDescrMethods<'py> for Bound<'py, PyArrayDescr> {

impl Sealed for Bound<'_, PyArrayDescr> {}

/// Weaker form of `Clone` for types that can be cloned while the GIL is held.
///
/// Any type that implements `Clone` can trivially implement `PyClone` by forwarding
/// to the `Clone::clone` method. However, some types (notably `PyObject`) can only
/// be safely cloned while the GIL is held, and therefore cannot implement `Clone`.
/// This trait provides a mechanism for performing a clone while the GIL is held, as
/// represented by the [`Python`] token provided as an argument to the [`py_clone`]
/// method. All API's in the `numpy` crate require the GIL to be held, so this weaker
/// alternative to `Clone` is a sufficient prerequisite for implementing the
/// [`Element`] trait.
///
/// # Implementing `PyClone`
/// Implementing this trait is trivial for most types, and simply requires defining
/// the `py_clone` method. The `vec_from_slice` and `array_from_view` methods have
/// default implementations that simply map the `py_clone` method to each item in
/// the collection, but types may want to override these implementations if there
/// is a more efficient way to perform the conversion. In particular, `Clone` types
/// may instead defer to the `ToOwned::to_owned` and `ArrayBase::to_owned` methods
/// for increased performance.
///
/// [`py_clone`]: Self::py_clone
pub trait PyClone: Sized {
/// Create a clone of the value while the GIL is guaranteed to be held.
fn py_clone(&self, py: Python<'_>) -> Self;

/// Create an owned copy of the slice while the GIL is guaranteed to be held.
///
/// Some types may provide implementations of this method that are more efficient
/// than simply mapping the `py_clone` method to each element in the slice.
#[inline]
fn vec_from_slice(py: Python<'_>, slc: &[Self]) -> Vec<Self> {
slc.iter().map(|elem| elem.py_clone(py)).collect()
}

/// Create an owned copy of the array while the GIL is guaranteed to be held.
///
/// Some types may provide implementations of this method that are more efficient
/// than simply mapping the `py_clone` method to each element in the view.
#[inline]
fn array_from_view<D>(
py: Python<'_>,
view: ::ndarray::ArrayView<'_, Self, D>,
) -> ::ndarray::Array<Self, D>
where
D: ::ndarray::Dimension,
{
view.map(|elem| elem.py_clone(py))
}
}

/// Represents that a type can be an element of `PyArray`.
///
/// Currently, only integer/float/complex/object types are supported. The [NumPy documentation][enumerated-types]
Expand Down Expand Up @@ -731,7 +681,7 @@ pub trait PyClone: Sized {
///
/// [enumerated-types]: https://numpy.org/doc/stable/reference/c-api/dtype.html#enumerated-types
/// [data-models]: https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models
pub unsafe trait Element: PyClone + Send {
pub unsafe trait Element: Sized + Send {
/// Flag that indicates whether this type is trivially copyable.
///
/// It should be set to true for all trivially copyable types (like scalar types
Expand All @@ -749,6 +699,33 @@ pub unsafe trait Element: PyClone + Send {

/// Returns the associated type descriptor ("dtype") for the given element type.
fn get_dtype_bound(py: Python<'_>) -> Bound<'_, PyArrayDescr>;

/// Create a clone of the value while the GIL is guaranteed to be held.
fn clone_ref(&self, py: Python<'_>) -> Self;

/// Create an owned copy of the slice while the GIL is guaranteed to be held.
///
/// Some types may provide implementations of this method that are more efficient
/// than simply mapping the `py_clone` method to each element in the slice.
#[inline]
fn vec_from_slice(py: Python<'_>, slc: &[Self]) -> Vec<Self> {
slc.iter().map(|elem| elem.clone_ref(py)).collect()
}

/// Create an owned copy of the array while the GIL is guaranteed to be held.
///
/// Some types may provide implementations of this method that are more efficient
/// than simply mapping the `py_clone` method to each element in the view.
#[inline]
fn array_from_view<D>(
py: Python<'_>,
view: ::ndarray::ArrayView<'_, Self, D>,
) -> ::ndarray::Array<Self, D>
where
D: ::ndarray::Dimension,
{
view.map(|elem| elem.clone_ref(py))
}
}

fn npy_int_type_lookup<T, T0, T1, T2>(npy_types: [NPY_TYPES; 3]) -> NPY_TYPES {
Expand Down Expand Up @@ -796,34 +773,33 @@ fn npy_int_type<T: Bounded + Zero + Sized + PartialEq>() -> NPY_TYPES {
}
}

// Implements `PyClone` for a type that implements `Clone`
macro_rules! impl_py_clone {
($ty:ty $(; [$param:ident $(: $bound:ident)?])?) => {
impl <$($param$(: $bound)*)?> $crate::dtype::PyClone for $ty {
#[inline]
fn py_clone(&self, _py: ::pyo3::Python<'_>) -> Self {
self.clone()
}
// Invoke within the `Element` impl for a `Clone` type to provide an efficient
// implementation of the cloning methods
macro_rules! clone_methods_impl {
($Self:ty) => {
#[inline]
fn clone_ref(&self, _py: ::pyo3::Python<'_>) -> $Self {
::std::clone::Clone::clone(self)
}

#[inline]
fn vec_from_slice(_py: ::pyo3::Python<'_>, slc: &[Self]) -> Vec<Self> {
slc.to_owned()
}
#[inline]
fn vec_from_slice(_py: ::pyo3::Python<'_>, slc: &[$Self]) -> Vec<$Self> {
::std::borrow::ToOwned::to_owned(slc)
}

#[inline]
fn array_from_view<D>(
_py: ::pyo3::Python<'_>,
view: ::ndarray::ArrayView<'_, Self, D>
) -> ::ndarray::Array<Self, D>
where
D: ::ndarray::Dimension
{
view.to_owned()
}
#[inline]
fn array_from_view<D>(
_py: ::pyo3::Python<'_>,
view: ::ndarray::ArrayView<'_, $Self, D>
) -> ::ndarray::Array<$Self, D>
where
D: ::ndarray::Dimension
{
::ndarray::ArrayView::to_owned(&view)
}
}
}
pub(crate) use impl_py_clone;
pub(crate) use clone_methods_impl;

macro_rules! impl_element_scalar {
(@impl: $ty:ty, $npy_type:expr $(,#[$meta:meta])*) => {
Expand All @@ -834,8 +810,9 @@ macro_rules! impl_element_scalar {
fn get_dtype_bound(py: Python<'_>) -> Bound<'_, PyArrayDescr> {
PyArrayDescr::from_npy_type(py, $npy_type)
}

clone_methods_impl!($ty);
}
impl_py_clone!($ty);
};
($ty:ty => $npy_type:ident $(,#[$meta:meta])*) => {
impl_element_scalar!(@impl: $ty, NPY_TYPES::$npy_type $(,#[$meta])*);
Expand Down Expand Up @@ -870,10 +847,9 @@ unsafe impl Element for bf16 {
.clone_ref(py)
.into_bound(py)
}
}

#[cfg(feature = "half")]
impl_py_clone!(bf16);
clone_methods_impl!(Self);
}

impl_element_scalar!(Complex32 => NPY_CFLOAT,
#[doc = "Complex type with `f32` components which maps to `numpy.csingle` (`numpy.complex64`)."]);
Expand All @@ -883,19 +859,17 @@ impl_element_scalar!(Complex64 => NPY_CDOUBLE,
#[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
impl_element_scalar!(usize, isize);

impl PyClone for PyObject {
#[inline]
fn py_clone(&self, py: Python<'_>) -> Self {
self.clone_ref(py)
}
}

unsafe impl Element for PyObject {
const IS_COPY: bool = false;

fn get_dtype_bound(py: Python<'_>) -> Bound<'_, PyArrayDescr> {
PyArrayDescr::object_bound(py)
}

#[inline]
fn clone_ref(&self, py: Python<'_>) -> Self {
Py::clone_ref(self, py)
}
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub use crate::convert::{IntoPyArray, NpyIndex, ToNpyDims, ToPyArray};
#[cfg(feature = "gil-refs")]
pub use crate::dtype::dtype;
pub use crate::dtype::{
dtype_bound, Complex32, Complex64, Element, PyArrayDescr, PyArrayDescrMethods, PyClone,
dtype_bound, Complex32, Complex64, Element, PyArrayDescr, PyArrayDescrMethods,
};
pub use crate::error::{BorrowError, FromVecError, NotContiguousError};
pub use crate::npyffi::{PY_ARRAY_API, PY_UFUNC_API};
Expand Down
50 changes: 4 additions & 46 deletions src/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use pyo3::{
};
use rustc_hash::FxHashMap;

use crate::dtype::{Element, PyArrayDescr, PyArrayDescrMethods, PyClone};
use crate::dtype::{clone_methods_impl, Element, PyArrayDescr, PyArrayDescrMethods};
use crate::npyffi::PyDataType_SET_ELSIZE;
use crate::npyffi::NPY_TYPES;

Expand Down Expand Up @@ -82,29 +82,8 @@ unsafe impl<const N: usize> Element for PyFixedString<N> {

unsafe { DTYPES.from_size(py, NPY_TYPES::NPY_STRING, b'|' as _, size_of::<Self>()) }
}
}

impl<const N: usize> PyClone for PyFixedString<N> {
#[inline]
fn py_clone(&self, _py: Python<'_>) -> Self {
*self
}

#[inline]
fn vec_from_slice(_py: Python<'_>, slc: &[Self]) -> Vec<Self> {
slc.to_owned()
}

#[inline]
fn array_from_view<D>(
_py: Python<'_>,
view: ::ndarray::ArrayView<'_, Self, D>,
) -> ::ndarray::Array<Self, D>
where
D: ::ndarray::Dimension,
{
view.to_owned()
}
clone_methods_impl!(Self);
}

/// A newtype wrapper around [`[PyUCS4; N]`][Py_UCS4] to handle [`str_` scalars][numpy-str] while satisfying coherence.
Expand Down Expand Up @@ -168,29 +147,6 @@ impl<const N: usize> From<[Py_UCS4; N]> for PyFixedUnicode<N> {
}
}

impl<const N: usize> PyClone for PyFixedUnicode<N> {
#[inline]
fn py_clone(&self, _py: Python<'_>) -> Self {
*self
}

#[inline]
fn vec_from_slice(_py: Python<'_>, slc: &[Self]) -> Vec<Self> {
slc.to_owned()
}

#[inline]
fn array_from_view<D>(
_py: Python<'_>,
view: ::ndarray::ArrayView<'_, Self, D>,
) -> ::ndarray::Array<Self, D>
where
D: ::ndarray::Dimension,
{
view.to_owned()
}
}

unsafe impl<const N: usize> Element for PyFixedUnicode<N> {
const IS_COPY: bool = true;

Expand All @@ -199,6 +155,8 @@ unsafe impl<const N: usize> Element for PyFixedUnicode<N> {

unsafe { DTYPES.from_size(py, NPY_TYPES::NPY_UNICODE, b'=' as _, size_of::<Self>()) }
}

clone_methods_impl!(Self);
}

struct TypeDescriptors {
Expand Down

0 comments on commit 71a30c2

Please sign in to comment.