From 0aa13c80063f7c62c96285313c6260b429060b1b Mon Sep 17 00:00:00 2001 From: Kevin Matlock Date: Mon, 28 Oct 2024 22:58:20 -0700 Subject: [PATCH] feat: Adds the Bound<'_, PyMappingProxy> type (#4644) * Mappingproxy (#1) Adds in the MappingProxy type. * Move over from `iter` to `try_iter`. * Added lifetime to `try_iter`, preventing need to clone when iterating. * Remove unneccessary borrow. * Add newsfragment * Newline to newsfragment. * Remove explicit lifetime, * Review comments (#2) * Addressing more comments * Remove extract_bound. * Remove extract methods. * Update comments for list return type. --------- Co-authored-by: Kevin Matlock --- newsfragments/4644.added.md | 1 + src/prelude.rs | 1 + src/sealed.rs | 5 +- src/types/mappingproxy.rs | 557 ++++++++++++++++++++++++++++++++++++ src/types/mod.rs | 2 + 5 files changed, 564 insertions(+), 2 deletions(-) create mode 100644 newsfragments/4644.added.md create mode 100644 src/types/mappingproxy.rs diff --git a/newsfragments/4644.added.md b/newsfragments/4644.added.md new file mode 100644 index 00000000000..4b4a277abf8 --- /dev/null +++ b/newsfragments/4644.added.md @@ -0,0 +1 @@ +New `PyMappingProxy` struct corresponing to the `mappingproxy` class in Python. diff --git a/src/prelude.rs b/src/prelude.rs index cc44a199611..7624d37a1e9 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -38,6 +38,7 @@ pub use crate::types::float::PyFloatMethods; pub use crate::types::frozenset::PyFrozenSetMethods; pub use crate::types::list::PyListMethods; pub use crate::types::mapping::PyMappingMethods; +pub use crate::types::mappingproxy::PyMappingProxyMethods; pub use crate::types::module::PyModuleMethods; pub use crate::types::sequence::PySequenceMethods; pub use crate::types::set::PySetMethods; diff --git a/src/sealed.rs b/src/sealed.rs index 62b47e131a7..cc835bee3b8 100644 --- a/src/sealed.rs +++ b/src/sealed.rs @@ -1,7 +1,7 @@ use crate::types::{ PyBool, PyByteArray, PyBytes, PyCapsule, PyComplex, PyDict, PyFloat, PyFrozenSet, PyList, - PyMapping, PyModule, PySequence, PySet, PySlice, PyString, PyTraceback, PyTuple, PyType, - PyWeakref, PyWeakrefProxy, PyWeakrefReference, + PyMapping, PyMappingProxy, PyModule, PySequence, PySet, PySlice, PyString, PyTraceback, + PyTuple, PyType, PyWeakref, PyWeakrefProxy, PyWeakrefReference, }; use crate::{ffi, Bound, PyAny, PyResult}; @@ -33,6 +33,7 @@ impl Sealed for Bound<'_, PyFloat> {} impl Sealed for Bound<'_, PyFrozenSet> {} impl Sealed for Bound<'_, PyList> {} impl Sealed for Bound<'_, PyMapping> {} +impl Sealed for Bound<'_, PyMappingProxy> {} impl Sealed for Bound<'_, PyModule> {} impl Sealed for Bound<'_, PySequence> {} impl Sealed for Bound<'_, PySet> {} diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs new file mode 100644 index 00000000000..fc28687c561 --- /dev/null +++ b/src/types/mappingproxy.rs @@ -0,0 +1,557 @@ +// Copyright (c) 2017-present PyO3 Project and Contributors + +use super::PyMapping; +use crate::err::PyResult; +use crate::ffi_ptr_ext::FfiPtrExt; +use crate::instance::Bound; +use crate::types::any::PyAnyMethods; +use crate::types::{PyAny, PyIterator, PyList}; +use crate::{ffi, Python}; + +use std::os::raw::c_int; + +/// Represents a Python `mappingproxy`. +#[repr(transparent)] +pub struct PyMappingProxy(PyAny); + +#[inline] +unsafe fn dict_proxy_check(op: *mut ffi::PyObject) -> c_int { + ffi::Py_IS_TYPE(op, std::ptr::addr_of_mut!(ffi::PyDictProxy_Type)) +} + +pyobject_native_type_core!( + PyMappingProxy, + pyobject_native_static_type_object!(ffi::PyDictProxy_Type), + #checkfunction=dict_proxy_check +); + +impl PyMappingProxy { + /// Creates a mappingproxy from an object. + pub fn new<'py>( + py: Python<'py>, + elements: &Bound<'py, PyMapping>, + ) -> Bound<'py, PyMappingProxy> { + unsafe { + ffi::PyDictProxy_New(elements.as_ptr()) + .assume_owned(py) + .downcast_into_unchecked() + } + } +} + +/// Implementation of functionality for [`PyMappingProxy`]. +/// +/// These methods are defined for the `Bound<'py, PyMappingProxy>` smart pointer, so to use method call +/// syntax these methods are separated into a trait, because stable Rust does not yet support +/// `arbitrary_self_types`. +#[doc(alias = "PyMappingProxy")] +pub trait PyMappingProxyMethods<'py, 'a>: crate::sealed::Sealed { + /// Checks if the mappingproxy is empty, i.e. `len(self) == 0`. + fn is_empty(&self) -> PyResult; + + /// Returns a list containing all keys in the mapping. + fn keys(&self) -> PyResult>; + + /// Returns a list containing all values in the mapping. + fn values(&self) -> PyResult>; + + /// Returns a list of tuples of all (key, value) pairs in the mapping. + fn items(&self) -> PyResult>; + + /// Returns `self` cast as a `PyMapping`. + fn as_mapping(&self) -> &Bound<'py, PyMapping>; + + /// Takes an object and returns an iterator for it. Returns an error if the object is not + /// iterable. + fn try_iter(&'a self) -> PyResult>; +} + +impl<'py, 'a> PyMappingProxyMethods<'py, 'a> for Bound<'py, PyMappingProxy> { + fn is_empty(&self) -> PyResult { + Ok(self.len()? == 0) + } + + #[inline] + fn keys(&self) -> PyResult> { + unsafe { + Ok(ffi::PyMapping_Keys(self.as_ptr()) + .assume_owned_or_err(self.py())? + .downcast_into_unchecked()) + } + } + + #[inline] + fn values(&self) -> PyResult> { + unsafe { + Ok(ffi::PyMapping_Values(self.as_ptr()) + .assume_owned_or_err(self.py())? + .downcast_into_unchecked()) + } + } + + #[inline] + fn items(&self) -> PyResult> { + unsafe { + Ok(ffi::PyMapping_Items(self.as_ptr()) + .assume_owned_or_err(self.py())? + .downcast_into_unchecked()) + } + } + + fn as_mapping(&self) -> &Bound<'py, PyMapping> { + unsafe { self.downcast_unchecked() } + } + + fn try_iter(&'a self) -> PyResult> { + Ok(BoundMappingProxyIterator { + iterator: PyIterator::from_object(self)?, + mappingproxy: self, + }) + } +} + +pub struct BoundMappingProxyIterator<'py, 'a> { + iterator: Bound<'py, PyIterator>, + mappingproxy: &'a Bound<'py, PyMappingProxy>, +} + +impl<'py> Iterator for BoundMappingProxyIterator<'py, '_> { + type Item = PyResult<(Bound<'py, PyAny>, Bound<'py, PyAny>)>; + + #[inline] + fn next(&mut self) -> Option { + self.iterator.next().map(|key| match key { + Ok(key) => match self.mappingproxy.get_item(&key) { + Ok(value) => Ok((key, value)), + Err(e) => Err(e), + }, + Err(e) => Err(e), + }) + } +} + +#[cfg(test)] +mod tests { + + use super::*; + use crate::types::dict::*; + use crate::Python; + use crate::{ + exceptions::PyKeyError, + types::{PyInt, PyTuple}, + }; + use std::collections::{BTreeMap, HashMap}; + + #[test] + fn test_new() { + Python::with_gil(|py| { + let pydict = [(7, 32)].into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, pydict.as_mapping()); + mappingproxy.get_item(7i32).unwrap(); + assert_eq!( + 32, + mappingproxy + .get_item(7i32) + .unwrap() + .extract::() + .unwrap() + ); + assert!(mappingproxy + .get_item(8i32) + .unwrap_err() + .is_instance_of::(py)); + }); + } + + #[test] + fn test_len() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + let dict = v.clone().into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + assert_eq!(mappingproxy.len().unwrap(), 0); + v.insert(7, 32); + let dict2 = v.clone().into_py_dict(py).unwrap(); + let mp2 = PyMappingProxy::new(py, dict2.as_mapping()); + assert_eq!(mp2.len().unwrap(), 1); + }); + } + + #[test] + fn test_contains() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + let dict = v.clone().into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + assert!(mappingproxy.contains(7i32).unwrap()); + assert!(!mappingproxy.contains(8i32).unwrap()); + }); + } + + #[test] + fn test_get_item() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + let dict = v.clone().into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + assert_eq!( + 32, + mappingproxy + .get_item(7i32) + .unwrap() + .extract::() + .unwrap() + ); + assert!(mappingproxy + .get_item(8i32) + .unwrap_err() + .is_instance_of::(py)); + }); + } + + #[test] + fn test_set_item_refcnt() { + Python::with_gil(|py| { + let cnt; + { + let none = py.None(); + cnt = none.get_refcnt(py); + let dict = [(10, none)].into_py_dict(py).unwrap(); + let _mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + } + { + assert_eq!(cnt, py.None().get_refcnt(py)); + } + }); + } + + #[test] + fn test_isempty() { + Python::with_gil(|py| { + let map: HashMap = HashMap::new(); + let dict = map.into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + assert!(mappingproxy.is_empty().unwrap()); + }); + } + + #[test] + fn test_keys() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + v.insert(8, 42); + v.insert(9, 123); + let dict = v.into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. + let mut key_sum = 0; + for el in mappingproxy.keys().unwrap().try_iter().unwrap() { + key_sum += el.unwrap().extract::().unwrap(); + } + assert_eq!(7 + 8 + 9, key_sum); + }); + } + + #[test] + fn test_values() { + Python::with_gil(|py| { + let mut v: HashMap = HashMap::new(); + v.insert(7, 32); + v.insert(8, 42); + v.insert(9, 123); + let dict = v.into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. + let mut values_sum = 0; + for el in mappingproxy.values().unwrap().try_iter().unwrap() { + values_sum += el.unwrap().extract::().unwrap(); + } + assert_eq!(32 + 42 + 123, values_sum); + }); + } + + #[test] + fn test_items() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + v.insert(8, 42); + v.insert(9, 123); + let dict = v.into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. + let mut key_sum = 0; + let mut value_sum = 0; + for res in mappingproxy.items().unwrap().try_iter().unwrap() { + let el = res.unwrap(); + let tuple = el.downcast::().unwrap(); + key_sum += tuple.get_item(0).unwrap().extract::().unwrap(); + value_sum += tuple.get_item(1).unwrap().extract::().unwrap(); + } + assert_eq!(7 + 8 + 9, key_sum); + assert_eq!(32 + 42 + 123, value_sum); + }); + } + + #[test] + fn test_iter() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + v.insert(8, 42); + v.insert(9, 123); + let dict = v.into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + let mut key_sum = 0; + let mut value_sum = 0; + for res in mappingproxy.try_iter().unwrap() { + let (key, value) = res.unwrap(); + key_sum += key.extract::().unwrap(); + value_sum += value.extract::().unwrap(); + } + assert_eq!(7 + 8 + 9, key_sum); + assert_eq!(32 + 42 + 123, value_sum); + }); + } + + #[test] + fn test_hashmap_into_python() { + Python::with_gil(|py| { + let mut map = HashMap::::new(); + map.insert(1, 1); + + let dict = map.clone().into_py_dict(py).unwrap(); + let py_map = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!(py_map.len().unwrap(), 1); + assert_eq!(py_map.get_item(1).unwrap().extract::().unwrap(), 1); + }); + } + + #[test] + fn test_hashmap_into_mappingproxy() { + Python::with_gil(|py| { + let mut map = HashMap::::new(); + map.insert(1, 1); + + let dict = map.clone().into_py_dict(py).unwrap(); + let py_map = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!(py_map.len().unwrap(), 1); + assert_eq!(py_map.get_item(1).unwrap().extract::().unwrap(), 1); + }); + } + + #[test] + fn test_btreemap_into_py() { + Python::with_gil(|py| { + let mut map = BTreeMap::::new(); + map.insert(1, 1); + + let dict = map.clone().into_py_dict(py).unwrap(); + let py_map = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!(py_map.len().unwrap(), 1); + assert_eq!(py_map.get_item(1).unwrap().extract::().unwrap(), 1); + }); + } + + #[test] + fn test_btreemap_into_mappingproxy() { + Python::with_gil(|py| { + let mut map = BTreeMap::::new(); + map.insert(1, 1); + + let dict = map.clone().into_py_dict(py).unwrap(); + let py_map = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!(py_map.len().unwrap(), 1); + assert_eq!(py_map.get_item(1).unwrap().extract::().unwrap(), 1); + }); + } + + #[test] + fn test_vec_into_mappingproxy() { + Python::with_gil(|py| { + let vec = vec![("a", 1), ("b", 2), ("c", 3)]; + let dict = vec.clone().into_py_dict(py).unwrap(); + let py_map = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!(py_map.len().unwrap(), 3); + assert_eq!(py_map.get_item("b").unwrap().extract::().unwrap(), 2); + }); + } + + #[test] + fn test_slice_into_mappingproxy() { + Python::with_gil(|py| { + let arr = [("a", 1), ("b", 2), ("c", 3)]; + + let dict = arr.into_py_dict(py).unwrap(); + let py_map = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!(py_map.len().unwrap(), 3); + assert_eq!(py_map.get_item("b").unwrap().extract::().unwrap(), 2); + }); + } + + #[test] + fn mappingproxy_as_mapping() { + Python::with_gil(|py| { + let mut map = HashMap::::new(); + map.insert(1, 1); + + let dict = map.clone().into_py_dict(py).unwrap(); + let py_map = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!(py_map.as_mapping().len().unwrap(), 1); + assert_eq!( + py_map + .as_mapping() + .get_item(1) + .unwrap() + .extract::() + .unwrap(), + 1 + ); + }); + } + + #[cfg(not(PyPy))] + fn abc_mappingproxy(py: Python<'_>) -> Bound<'_, PyMappingProxy> { + let mut map = HashMap::<&'static str, i32>::new(); + map.insert("a", 1); + map.insert("b", 2); + map.insert("c", 3); + let dict = map.clone().into_py_dict(py).unwrap(); + PyMappingProxy::new(py, dict.as_mapping()) + } + + #[test] + #[cfg(not(PyPy))] + fn mappingproxy_keys_view() { + Python::with_gil(|py| { + let mappingproxy = abc_mappingproxy(py); + let keys = mappingproxy.call_method0("keys").unwrap(); + assert!(keys.is_instance(&py.get_type::()).unwrap()); + }) + } + + #[test] + #[cfg(not(PyPy))] + fn mappingproxy_values_view() { + Python::with_gil(|py| { + let mappingproxy = abc_mappingproxy(py); + let values = mappingproxy.call_method0("values").unwrap(); + assert!(values.is_instance(&py.get_type::()).unwrap()); + }) + } + + #[test] + #[cfg(not(PyPy))] + fn mappingproxy_items_view() { + Python::with_gil(|py| { + let mappingproxy = abc_mappingproxy(py); + let items = mappingproxy.call_method0("items").unwrap(); + assert!(items.is_instance(&py.get_type::()).unwrap()); + }) + } + + #[test] + fn get_value_from_mappingproxy_of_strings() { + Python::with_gil(|py: Python<'_>| { + let mut map = HashMap::new(); + map.insert("first key".to_string(), "first value".to_string()); + map.insert("second key".to_string(), "second value".to_string()); + map.insert("third key".to_string(), "third value".to_string()); + + let dict = map.clone().into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!( + map.into_iter().collect::>(), + mappingproxy + .try_iter() + .unwrap() + .map(|object| { + let tuple = object.unwrap(); + ( + tuple.0.extract::().unwrap(), + tuple.1.extract::().unwrap(), + ) + }) + .collect::>() + ); + }) + } + + #[test] + fn get_value_from_mappingproxy_of_integers() { + Python::with_gil(|py: Python<'_>| { + const LEN: usize = 10_000; + let items: Vec<(usize, usize)> = (1..LEN).map(|i| (i, i - 1)).collect(); + + let dict = items.clone().into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + + assert_eq!( + items, + mappingproxy + .clone() + .try_iter() + .unwrap() + .map(|object| { + let tuple = object.unwrap(); + ( + tuple + .0 + .downcast::() + .unwrap() + .extract::() + .unwrap(), + tuple + .1 + .downcast::() + .unwrap() + .extract::() + .unwrap(), + ) + }) + .collect::>() + ); + for index in 1..LEN { + assert_eq!( + mappingproxy + .clone() + .get_item(index) + .unwrap() + .extract::() + .unwrap(), + index - 1 + ); + } + }) + } + + #[test] + fn iter_mappingproxy_nosegv() { + Python::with_gil(|py| { + const LEN: usize = 10_000_000; + let items = (0..LEN as u64).map(|i| (i, i * 2)); + + let dict = items.clone().into_py_dict(py).unwrap(); + let mappingproxy = PyMappingProxy::new(py, dict.as_mapping()); + + let mut sum = 0; + for result in mappingproxy.try_iter().unwrap() { + let (k, _v) = result.unwrap(); + let i: u64 = k.extract().unwrap(); + sum += i; + } + assert_eq!(sum, 49_999_995_000_000); + }) + } +} diff --git a/src/types/mod.rs b/src/types/mod.rs index c074196ccc1..d84f099e773 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -28,6 +28,7 @@ pub use self::function::PyFunction; pub use self::iterator::PyIterator; pub use self::list::{PyList, PyListMethods}; pub use self::mapping::{PyMapping, PyMappingMethods}; +pub use self::mappingproxy::PyMappingProxy; pub use self::memoryview::PyMemoryView; pub use self::module::{PyModule, PyModuleMethods}; pub use self::none::PyNone; @@ -248,6 +249,7 @@ mod function; pub(crate) mod iterator; pub(crate) mod list; pub(crate) mod mapping; +pub(crate) mod mappingproxy; mod memoryview; pub(crate) mod module; mod none;