From 5d47c4ae4ca9b1eb4112fbc5273c3706365f59d9 Mon Sep 17 00:00:00 2001 From: JRRudy1 <31031841+JRRudy1@users.noreply.github.com> Date: Sat, 1 Jun 2024 16:09:14 -0500 Subject: [PATCH] Added `ToPyObject` and `IntoPy` impls for `PyBackedStr`. (#4205) * Added `ToPyObject` and `Into` impls for `PyBackedStr`. * Create 4205.added.md * Added attributes limiting the `ToPyObject` and `IntoPy` impls to the case where `cfg(any(Py_3_10, not(Py_LIMITED_API)))`. When this cfg does not apply, the conversion is less trivial since the `storage` is actually `PyBytes`, not `PyString`. * Fixed imports format. * Updated the `ToPyObject` and `IntoPy` impls to support the `cfg(not(any(Py_3_10, not(Py_LIMITED_API))))` case by converting the `PyBytes` back to `PyString`. * Added `ToPyObject` and `IntoPy` impls for `PyBackedBytes`. * Added tests for the `PyBackedBytes` conversion impls. * Updated newsfragment entry to include the `PyBackedBytes` impls. * Changed the `IntoPy` and `ToPyObject` impls for `PyBackedBytes` to produce `PyBytes` regardless of the backing variant. Updated tests to demonstrate this. * retrigger checks * Updated `PyBackedStr` conversion tests to extract the result as a `PyBackedStr` instead of `&str` since the latter is not supported under some `cfg`'s. * Fixed `IntoPy for PyBackedBytes` impl to create `bytes` for both storage types as intended. Updated test to properly catch the error. --------- Co-authored-by: jrudolph --- newsfragments/4205.added.md | 1 + src/pybacked.rs | 94 ++++++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 newsfragments/4205.added.md diff --git a/newsfragments/4205.added.md b/newsfragments/4205.added.md new file mode 100644 index 00000000..86ce9fc3 --- /dev/null +++ b/newsfragments/4205.added.md @@ -0,0 +1 @@ +Added `ToPyObject` and `IntoPy` impls for `PyBackedStr` and `PyBackedBytes`. diff --git a/src/pybacked.rs b/src/pybacked.rs index ed68ea52..f6a0f99f 100644 --- a/src/pybacked.rs +++ b/src/pybacked.rs @@ -7,7 +7,7 @@ use crate::{ any::PyAnyMethods, bytearray::PyByteArrayMethods, bytes::PyBytesMethods, string::PyStringMethods, PyByteArray, PyBytes, PyString, }, - Bound, DowncastError, FromPyObject, Py, PyAny, PyErr, PyResult, + Bound, DowncastError, FromPyObject, IntoPy, Py, PyAny, PyErr, PyResult, Python, ToPyObject, }; /// A wrapper around `str` where the storage is owned by a Python `bytes` or `str` object. @@ -85,6 +85,28 @@ impl FromPyObject<'_> for PyBackedStr { } } +impl ToPyObject for PyBackedStr { + #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] + fn to_object(&self, py: Python<'_>) -> Py { + self.storage.clone_ref(py) + } + #[cfg(not(any(Py_3_10, not(Py_LIMITED_API))))] + fn to_object(&self, py: Python<'_>) -> Py { + PyString::new_bound(py, self).into_any().unbind() + } +} + +impl IntoPy> for PyBackedStr { + #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] + fn into_py(self, _py: Python<'_>) -> Py { + self.storage + } + #[cfg(not(any(Py_3_10, not(Py_LIMITED_API))))] + fn into_py(self, py: Python<'_>) -> Py { + PyString::new_bound(py, &self).into_any().unbind() + } +} + /// A wrapper around `[u8]` where the storage is either owned by a Python `bytes` object, or a Rust `Box<[u8]>`. /// /// This type gives access to the underlying data via a `Deref` implementation. @@ -181,6 +203,24 @@ impl FromPyObject<'_> for PyBackedBytes { } } +impl ToPyObject for PyBackedBytes { + fn to_object(&self, py: Python<'_>) -> Py { + match &self.storage { + PyBackedBytesStorage::Python(bytes) => bytes.to_object(py), + PyBackedBytesStorage::Rust(bytes) => PyBytes::new_bound(py, bytes).into_any().unbind(), + } + } +} + +impl IntoPy> for PyBackedBytes { + fn into_py(self, py: Python<'_>) -> Py { + match self.storage { + PyBackedBytesStorage::Python(bytes) => bytes.into_any(), + PyBackedBytesStorage::Rust(bytes) => PyBytes::new_bound(py, &bytes).into_any().unbind(), + } + } +} + macro_rules! impl_traits { ($slf:ty, $equiv:ty) => { impl std::fmt::Debug for $slf { @@ -288,6 +328,30 @@ mod test { }); } + #[test] + fn py_backed_str_to_object() { + Python::with_gil(|py| { + let orig_str = PyString::new_bound(py, "hello"); + let py_backed_str = orig_str.extract::().unwrap(); + let new_str = py_backed_str.to_object(py); + assert_eq!(new_str.extract::(py).unwrap(), "hello"); + #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] + assert!(new_str.is(&orig_str)); + }); + } + + #[test] + fn py_backed_str_into_py() { + Python::with_gil(|py| { + let orig_str = PyString::new_bound(py, "hello"); + let py_backed_str = orig_str.extract::().unwrap(); + let new_str = py_backed_str.into_py(py); + assert_eq!(new_str.extract::(py).unwrap(), "hello"); + #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] + assert!(new_str.is(&orig_str)); + }); + } + #[test] fn py_backed_bytes_empty() { Python::with_gil(|py| { @@ -324,6 +388,34 @@ mod test { }); } + #[test] + fn py_backed_bytes_into_py() { + Python::with_gil(|py| { + let orig_bytes = PyBytes::new_bound(py, b"abcde"); + let py_backed_bytes = PyBackedBytes::from(orig_bytes.clone()); + assert!(py_backed_bytes.to_object(py).is(&orig_bytes)); + assert!(py_backed_bytes.into_py(py).is(&orig_bytes)); + }); + } + + #[test] + fn rust_backed_bytes_into_py() { + Python::with_gil(|py| { + let orig_bytes = PyByteArray::new_bound(py, b"abcde"); + let rust_backed_bytes = PyBackedBytes::from(orig_bytes); + assert!(matches!( + rust_backed_bytes.storage, + PyBackedBytesStorage::Rust(_) + )); + let to_object = rust_backed_bytes.to_object(py).into_bound(py); + assert!(&to_object.is_exact_instance_of::()); + assert_eq!(&to_object.extract::().unwrap(), b"abcde"); + let into_py = rust_backed_bytes.into_py(py).into_bound(py); + assert!(&into_py.is_exact_instance_of::()); + assert_eq!(&into_py.extract::().unwrap(), b"abcde"); + }); + } + #[test] fn test_backed_types_send_sync() { fn is_send() {}