From f8fcfacfdbee956789c272df6a28b7b9956e0294 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Thu, 2 Jun 2022 12:09:00 +0100 Subject: [PATCH 01/34] Write mappingproxy tests --- tests/test_mappingproxy.rs | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/test_mappingproxy.rs diff --git a/tests/test_mappingproxy.rs b/tests/test_mappingproxy.rs new file mode 100644 index 00000000000..db373b36cd0 --- /dev/null +++ b/tests/test_mappingproxy.rs @@ -0,0 +1,56 @@ +use std::collections::HashMap; + +use pyo3::prelude::Python; +use pyo3::types::{IntoPyMappingProxy, PyString, PyInt}; + +const LEN: usize = 10_000_000; + +#[test] +fn get_value_from_mappingproxy_of_strings(){ + let gil = Python::acquire_gil(); + let py = gil.python(); + + let map = HashMap::new(); + map.insert("first key", "first value"); + map.insert("second key", "second value"); + map.insert("third key", "third value"); + + let mappingproxy = map.iter().into_py_mappingproxy(); + + assert_eq!(map.iter(), mappingproxy.iter().map(|object| object.downcast::().unwrap().to_str().unwrap())); +} + +#[test] +fn get_value_from_mappingproxy_of_integers(){ + let gil = Python::acquire_gil(); + let py = gil.python(); + + let map = (0..LEN).map(|i| (i, i - 1)); + let mappingproxy = map.into_py_mappingproxy(py); + assert_eq!( + map, + mappingproxy.iter().map( + |object| object.downcast::().unwrap().extract::().unwrap() + ) + ); + for index in 0..LEN { + assert_eq!( + mappingproxy.get_item(index).unwrap().extract::().unwrap(), + index - 1 + ); + } +} + +#[test] +fn iter_mappingproxy_nosegv() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let mappingproxy = (0..LEN as u64).map(|i| (i, i * 2)).into_py_mappingproxy(py); + + let mut sum = 0; + for (k, _v) in mappingproxy.iter() { + let i: u64 = k.extract().unwrap(); + sum += i; + } + assert_eq!(sum, 49_999_995_000_000); +} From e8e391338ba202bd20e3c9a7d2ac05939bfcbbb7 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Thu, 2 Jun 2022 19:34:25 +0100 Subject: [PATCH 02/34] Amend tests to reflect proposed API --- tests/test_mappingproxy.rs | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/test_mappingproxy.rs b/tests/test_mappingproxy.rs index db373b36cd0..945dd432ab3 100644 --- a/tests/test_mappingproxy.rs +++ b/tests/test_mappingproxy.rs @@ -10,14 +10,23 @@ fn get_value_from_mappingproxy_of_strings(){ let gil = Python::acquire_gil(); let py = gil.python(); - let map = HashMap::new(); + let mut map = HashMap::new(); map.insert("first key", "first value"); map.insert("second key", "second value"); map.insert("third key", "third value"); - let mappingproxy = map.iter().into_py_mappingproxy(); + let mappingproxy = map.iter().into_py_mappingproxy(py).unwrap(); - assert_eq!(map.iter(), mappingproxy.iter().map(|object| object.downcast::().unwrap().to_str().unwrap())); + assert_eq!( + map.into_iter().collect::>(), + mappingproxy.iter().map( + |object| + ( + object.0.downcast::().unwrap().to_str().unwrap(), + object.1.downcast::().unwrap().to_str().unwrap() + ) + ).collect::>() + ); } #[test] @@ -25,13 +34,17 @@ fn get_value_from_mappingproxy_of_integers(){ let gil = Python::acquire_gil(); let py = gil.python(); - let map = (0..LEN).map(|i| (i, i - 1)); - let mappingproxy = map.into_py_mappingproxy(py); + let items = (0..LEN).map(|i| (i, i - 1)).collect::>(); + let mappingproxy = items.to_vec().into_py_mappingproxy(py).unwrap(); assert_eq!( - map, + items, mappingproxy.iter().map( - |object| object.downcast::().unwrap().extract::().unwrap() - ) + |object| + ( + object.0.downcast::().unwrap().extract::().unwrap(), + object.1.downcast::().unwrap().extract::().unwrap() + ) + ).collect::>() ); for index in 0..LEN { assert_eq!( @@ -45,7 +58,7 @@ fn get_value_from_mappingproxy_of_integers(){ fn iter_mappingproxy_nosegv() { let gil = Python::acquire_gil(); let py = gil.python(); - let mappingproxy = (0..LEN as u64).map(|i| (i, i * 2)).into_py_mappingproxy(py); + let mappingproxy = (0..LEN as u64).map(|i| (i, i * 2)).into_py_mappingproxy(py).unwrap(); let mut sum = 0; for (k, _v) in mappingproxy.iter() { From bd5b89ddf2074254092a030550d5bc636154578e Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Thu, 2 Jun 2022 19:35:09 +0100 Subject: [PATCH 03/34] Update ffi --- pyo3-ffi/src/descrobject.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pyo3-ffi/src/descrobject.rs b/pyo3-ffi/src/descrobject.rs index 253b40dc4ab..7e0fdab7732 100644 --- a/pyo3-ffi/src/descrobject.rs +++ b/pyo3-ffi/src/descrobject.rs @@ -17,6 +17,13 @@ pub struct PyGetSetDef { pub closure: *mut c_void, } +#[repr(C)] +#[derive(Copy, Clone, Debug)] +pub struct PyDictProxyObject { + pub ob_base: PyObject, + pub mapping: PyObject +} + // skipped non-limited wrapperfunc // skipped non-limited wrapperfunc_kwds // skipped non-limited struct wrapperbase From 4757e6d44b006261bfe292e295420092bce5f22b Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 5 Jun 2022 15:39:03 +0100 Subject: [PATCH 04/34] Prevent integer underflow in test --- src/types/mod.rs | 2 ++ tests/test_mappingproxy.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/types/mod.rs b/src/types/mod.rs index e4f72577db6..f047cb14cf8 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -16,6 +16,7 @@ pub use self::datetime::{ PyTzInfoAccess, }; pub use self::dict::{IntoPyDict, PyDict}; +pub use self::mappingproxy::{IntoPyMappingProxy, PyMappingProxy}; pub use self::floatob::PyFloat; #[cfg(all(not(Py_LIMITED_API), not(PyPy)))] pub use self::frame::PyFrame; @@ -275,6 +276,7 @@ mod function; mod iterator; mod list; mod mapping; +mod mappingproxy; mod module; mod num; mod sequence; diff --git a/tests/test_mappingproxy.rs b/tests/test_mappingproxy.rs index 945dd432ab3..1ab76ba56ed 100644 --- a/tests/test_mappingproxy.rs +++ b/tests/test_mappingproxy.rs @@ -34,7 +34,7 @@ fn get_value_from_mappingproxy_of_integers(){ let gil = Python::acquire_gil(); let py = gil.python(); - let items = (0..LEN).map(|i| (i, i - 1)).collect::>(); + let items: Vec<(usize, usize)> = (1..LEN).map(|i| (i, i - 1)).collect(); let mappingproxy = items.to_vec().into_py_mappingproxy(py).unwrap(); assert_eq!( items, @@ -46,7 +46,7 @@ fn get_value_from_mappingproxy_of_integers(){ ) ).collect::>() ); - for index in 0..LEN { + for index in 1..LEN { assert_eq!( mappingproxy.get_item(index).unwrap().extract::().unwrap(), index - 1 From d99220063b800e3c4096c719399f13f8bcc68aa6 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 5 Jun 2022 15:52:51 +0100 Subject: [PATCH 05/34] Add support for mappingproxy objects --- pyo3-ffi/src/descrobject.rs | 2 +- pyo3-ffi/src/lib.rs | 1 + src/types/dict.rs | 36 ++- src/types/mappingproxy.rs | 538 ++++++++++++++++++++++++++++++++++++ 4 files changed, 565 insertions(+), 12 deletions(-) create mode 100644 src/types/mappingproxy.rs diff --git a/pyo3-ffi/src/descrobject.rs b/pyo3-ffi/src/descrobject.rs index 7e0fdab7732..1e5cd2cc7bb 100644 --- a/pyo3-ffi/src/descrobject.rs +++ b/pyo3-ffi/src/descrobject.rs @@ -21,7 +21,7 @@ pub struct PyGetSetDef { #[derive(Copy, Clone, Debug)] pub struct PyDictProxyObject { pub ob_base: PyObject, - pub mapping: PyObject + pub mapping: *mut PyObject } // skipped non-limited wrapperfunc diff --git a/pyo3-ffi/src/lib.rs b/pyo3-ffi/src/lib.rs index 9eabd846c69..ad7366ad154 100644 --- a/pyo3-ffi/src/lib.rs +++ b/pyo3-ffi/src/lib.rs @@ -264,6 +264,7 @@ macro_rules! opaque_struct { }; } +#[macro_export] macro_rules! addr_of_mut_shim { ($place:expr) => {{ #[cfg(addr_of)] diff --git a/src/types/dict.rs b/src/types/dict.rs index bd75d89722b..8796092d037 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -3,7 +3,7 @@ use super::PyMapping; use crate::err::{self, PyErr, PyResult}; use crate::ffi::Py_ssize_t; -use crate::types::{PyAny, PyList}; +use crate::types::{PyAny, PyList, PyMappingProxy}; #[cfg(not(PyPy))] use crate::IntoPyPointer; use crate::{ffi, AsPyPointer, FromPyObject, IntoPy, PyObject, PyTryFrom, Python, ToPyObject}; @@ -444,12 +444,19 @@ where S: hash::BuildHasher + Default, { fn extract(ob: &'source PyAny) -> Result { - let dict = ::try_from(ob)?; - let mut ret = HashMap::with_capacity_and_hasher(dict.len(), S::default()); - for (k, v) in dict.iter() { - ret.insert(K::extract(k)?, V::extract(v)?); + match ::try_from(ob) { + Ok(dict) => { + let mut ret = HashMap::with_capacity_and_hasher(dict.len(), S::default()); + for (k, v) in dict.iter() { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } + Err(_) => { + let mappingproxy = ::try_from(ob)?; + mappingproxy.copy()?.extract() + } } - Ok(ret) } } @@ -459,12 +466,19 @@ where V: FromPyObject<'source>, { fn extract(ob: &'source PyAny) -> Result { - let dict = ::try_from(ob)?; - let mut ret = BTreeMap::new(); - for (k, v) in dict.iter() { - ret.insert(K::extract(k)?, V::extract(v)?); + match ::try_from(ob) { + Ok(dict) => { + let mut ret = BTreeMap::new(); + for (k, v) in dict.iter() { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } + Err(_) => { + let mappingproxy = ::try_from(ob)?; + mappingproxy.copy()?.extract() + } } - Ok(ret) } } diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs new file mode 100644 index 00000000000..1ee8d5c3326 --- /dev/null +++ b/src/types/mappingproxy.rs @@ -0,0 +1,538 @@ +// Copyright (c) 2017-present PyO3 Project and Contributors + +use super::PyMapping; +use crate::err::{PyResult}; +use crate::ffi::{PyDictProxy_Type, PyObject_TypeCheck}; +use crate::ffi::addr_of_mut_shim; +use crate::types::{IntoPyDict, PyAny, PyDict, PyList, PyString, PyIterator, PySequence}; +use crate::types::dict::PyDictItem; +#[cfg(test)] +use crate::types::dict::{PyDictKeys, PyDictValues, PyDictItems, PyDictItem}; +#[cfg(not(PyPy))] +use crate::{ffi, AsPyPointer, PyObject, PyTryFrom, Python, ToPyObject}; +use std::os::raw::c_int; + + +#[inline] +#[allow(non_snake_case)] +unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { + PyObject_TypeCheck(object, addr_of_mut_shim!(PyDictProxy_Type)) +} + +/// Represents a Python `mappingproxy`. +#[repr(transparent)] +pub struct PyMappingProxy(PyAny); + +pyobject_native_type!( + PyMappingProxy, + ffi::PyDictProxyObject, + ffi::PyDictProxy_Type, + #checkfunction=PyDictProxy_Check +); + +impl PyMappingProxy { + /// Creates a mappingproxy from an object. + pub fn new(py: Python<'_>, elements: impl IntoPyDict) -> PyResult<&'_ PyMappingProxy> { + unsafe { + let dict = elements.into_py_dict(py); + let proxy = ffi::PyDictProxy_New(dict.as_ptr()); + py.from_owned_ptr_or_err::(proxy) + } + } + + /// Creates a new mappingproxy from the sequence given. + /// + /// The sequence must consist of `(PyObject, PyObject)`. + /// + /// Returns an error on invalid input. In the case of key collisions, + /// this keeps the last entry seen. + #[cfg(not(PyPy))] + pub fn from_sequence(py: Python<'_>, seq: PyObject) -> PyResult<&PyMappingProxy> { + unsafe { + let dict = py.from_owned_ptr::(PyDict::from_sequence(py, seq)?.as_ptr()); + py.from_owned_ptr_or_err(ffi::PyDictProxy_New(dict.as_ptr())) + } + } + + /// Returns a new mappingproxy that contains the same key-value pairs as self. + /// + /// This is equivalent to the Python expression `self.copy()`. + pub fn copy(&self) -> PyResult<&PyDict> { + unsafe { + let dict = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "copy").as_ptr()); + self.py().from_owned_ptr_or_err(dict) + } + } + + /// Checks if the mappingproxy is empty, i.e. `len(self) == 0`. + pub fn is_empty(&self) -> bool { + self.len().unwrap() == 0 + } + + /// Gets an item from the mappingproxy. + /// + /// Returns `None` if the item is not present, or if an error occurs. + /// + /// To get a `KeyError` for non-existing keys, use `PyAny::get_item`. + pub fn get_item(&self, key: K) -> Option<&PyAny> + where + K: ToPyObject, + { + PyAny::get_item(self, key).ok() + } + + + /// Returns a list of mappingproxy keys. + /// + /// This is equivalent to the Python expression `list(mappingproxy.keys())`. + pub fn keys(&self) -> &PyList { + unsafe { + let keys = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "keys").as_ptr()); + self.py() + .from_owned_ptr::(keys).list().unwrap() + } + } + + /// Returns a list of mappingproxy values. + /// + /// This is equivalent to the Python expression `list(mappingproxy.values())`. + pub fn values(&self) -> &PyList { + unsafe { + let values = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "values").as_ptr()); + self.py() + .from_owned_ptr::(values).list().unwrap() + } + } + + /// Returns a list of mappingproxy items. + /// + /// This is equivalent to the Python expression `list(mappingproxy.items())`. + pub fn items(&self) -> &PyList { + unsafe { + let items = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "items").as_ptr()); + self.py() + .from_owned_ptr::(items).list().unwrap() + } + } + + /// + /// Returns an iterator of `(key, value)` pairs in this mappingproxy. + pub fn iter(&self) -> PyMappingProxyIterator<'_> { + IntoIterator::into_iter(self) + } + + /// Returns `self` cast as a `PyMapping`. + pub fn as_mapping(&self) -> &PyMapping { + unsafe { PyMapping::try_from_unchecked(self) } + } +} + +pub struct PyMappingProxyIterator<'py> { + iterator: &'py PyIterator, + mappingproxy: &'py PyMappingProxy +} + +impl<'py> Iterator for PyMappingProxyIterator<'py> { + type Item = (&'py PyAny, &'py PyAny); + + #[inline] + fn next(&mut self) -> Option { + self.iterator.next() + .map(Result::unwrap) + .and_then( + |key| self.mappingproxy.get_item(key).map(|value| (key, value)) + ) + } +} + +impl<'a> std::iter::IntoIterator for &'a PyMappingProxy { + type Item = (&'a PyAny, &'a PyAny); + type IntoIter = PyMappingProxyIterator<'a>; + + fn into_iter(self) -> Self::IntoIter { + PyMappingProxyIterator { + iterator: PyIterator::from_object(self.py(), self).unwrap(), + mappingproxy: self + } + } +} + +/// Conversion trait that allows a sequence of tuples to be converted into `PyMappingProxy` +/// Primary use case for this trait is `call` and `call_method` methods as keywords argument. +pub trait IntoPyMappingProxy { + /// Converts self into a `PyMappingProxy` object pointer. Whether pointer owned or borrowed + /// depends on implementation. + fn into_py_mappingproxy(self, py: Python<'_>) -> PyResult<&PyMappingProxy>; +} + +impl IntoPyMappingProxy for I +where + T: PyDictItem, + I: IntoIterator, +{ + fn into_py_mappingproxy(self, py: Python<'_>) -> PyResult<&PyMappingProxy> { + PyMappingProxy::new(py, self) + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[cfg(not(PyPy))] + use crate::{types::PyList, PyTypeInfo}; + use crate::{types::PyTuple, IntoPy, PyObject, PyTryFrom, Python, ToPyObject}; + use std::collections::{BTreeMap, HashMap}; + + #[test] + fn test_new() { + Python::with_gil(|py| { + let mappingproxy = [(7, 32)].into_py_mappingproxy(py).unwrap(); + assert_eq!(32, mappingproxy.get_item(7i32).unwrap().extract::().unwrap()); + assert!(mappingproxy.get_item(8i32).is_none()); + let map: HashMap = [(7, 32)].iter().cloned().collect(); + assert_eq!(map, mappingproxy.extract().unwrap()); + let map: BTreeMap = [(7, 32)].iter().cloned().collect(); + assert_eq!(map, mappingproxy.extract().unwrap()); + }); + } + + #[test] + #[cfg(not(PyPy))] + fn test_from_sequence() { + Python::with_gil(|py| { + let items = PyList::new(py, &vec![("a", 1), ("b", 2)]); + let mappingproxy = PyMappingProxy::from_sequence(py, items.to_object(py)).unwrap(); + assert_eq!(1, mappingproxy.get_item("a").unwrap().extract::().unwrap()); + assert_eq!(2, mappingproxy.get_item("b").unwrap().extract::().unwrap()); + let map: HashMap<&str, i32> = [("a", 1), ("b", 2)].iter().cloned().collect(); + assert_eq!(map, mappingproxy.extract().unwrap()); + let map: BTreeMap<&str, i32> = [("a", 1), ("b", 2)].iter().cloned().collect(); + assert_eq!(map, mappingproxy.extract().unwrap()); + }); + } + + #[test] + #[cfg(not(PyPy))] + fn test_from_sequence_err() { + Python::with_gil(|py| { + let items = PyList::new(py, &vec!["a", "b"]); + assert!(PyMappingProxy::from_sequence(py, items.to_object(py)).is_err()); + }); + } + + #[test] + fn test_copy() { + Python::with_gil(|py| { + let mappingproxy = [(7, 32)].into_py_mappingproxy(py).unwrap(); + + let new_dict = mappingproxy.copy().unwrap(); + assert_eq!(32, new_dict.get_item(7i32).unwrap().extract::().unwrap()); + assert!(new_dict.get_item(8i32).is_none()); + }); + } + + #[test] + fn test_len() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + let mappingproxy = v.clone().into_py_mappingproxy(py).unwrap(); + assert_eq!(mappingproxy.len().unwrap(), 0); + v.insert(7, 32); + let mp2 = v.into_py_mappingproxy(py).unwrap(); + assert_eq!(mp2.len().unwrap(), 1); + }); + } + + #[test] + fn test_contains() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + let mappingproxy = v.into_py_mappingproxy(py).unwrap(); + 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 mappingproxy = v.into_py_mappingproxy(py).unwrap(); + assert_eq!(32, mappingproxy.get_item(7i32).unwrap().extract::().unwrap()); + assert!(mappingproxy.get_item(8i32).is_none()); + }); + } + + #[test] + fn test_set_item_refcnt() { + Python::with_gil(|py| { + let cnt; + { + let _pool = unsafe { crate::GILPool::new() }; + let none = py.None(); + cnt = none.get_refcnt(py); + let _mapping_proxy = [(10, none)].into_py_mappingproxy(py); + } + { + assert_eq!(cnt, py.None().get_refcnt(py)); + } + }); + } + + #[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 mappingproxy = v.into_py_mappingproxy(py).unwrap(); + // 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 el in mappingproxy.items().iter() { + let tuple = el.cast_as::().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_keys() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + v.insert(8, 42); + v.insert(9, 123); + let mappingproxy = v.into_py_mappingproxy(py).unwrap(); + // 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().iter() { + key_sum += el.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 mappingproxy = v.into_py_mappingproxy(py).unwrap(); + // 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().iter() { + values_sum += el.extract::().unwrap(); + } + assert_eq!(32 + 42 + 123, values_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 mappingproxy = v.into_py_mappingproxy(py).unwrap(); + let mut key_sum = 0; + let mut value_sum = 0; + for (key, value) in mappingproxy.iter() { + 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_into_iter() { + Python::with_gil(|py| { + let mut v = HashMap::new(); + v.insert(7, 32); + v.insert(8, 42); + v.insert(9, 123); + let mappingproxy = v.into_py_mappingproxy(py).unwrap(); + let mut key_sum = 0; + let mut value_sum = 0; + for (key, value) in mappingproxy { + 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_to_python() { + Python::with_gil(|py| { + let mut map = HashMap::::new(); + map.insert(1, 1); + + let py_map = map.clone().into_py_mappingproxy(py).unwrap(); + + assert_eq!(py_map.len().unwrap(), 1); + assert_eq!(py_map.get_item(1).unwrap().extract::().unwrap(), 1); + assert_eq!(map, py_map.extract().unwrap()); + }); + } + + #[test] + fn test_btreemap_to_python() { + Python::with_gil(|py| { + let mut map = BTreeMap::::new(); + map.insert(1, 1); + + let py_map = map.clone().into_py_mappingproxy(py).unwrap(); + + assert_eq!(py_map.len().unwrap(), 1); + assert_eq!(py_map.get_item(1).unwrap().extract::().unwrap(), 1); + assert_eq!(map, py_map.extract().unwrap()); + }); + } + + #[test] + fn test_hashmap_into_python() { + Python::with_gil(|py| { + let mut map = HashMap::::new(); + map.insert(1, 1); + + let py_map = map.into_py_mappingproxy(py).unwrap(); + + 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 py_map = map.into_py_mappingproxy(py).unwrap(); + + 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 py_map = map.clone().into_py_mappingproxy(py).unwrap(); + + 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 py_map = map.into_py_mappingproxy(py).unwrap(); + + 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 py_map = vec.into_py_mappingproxy(py).unwrap(); + + 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 py_map = arr.into_py_mappingproxy(py).unwrap(); + + 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 py_map = map.into_py_mappingproxy(py).unwrap(); + + 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<'_>) -> &PyMappingProxy { + let mut map = HashMap::<&'static str, i32>::new(); + map.insert("a", 1); + map.insert("b", 2); + map.insert("c", 3); + map.into_py_mappingproxy(py).unwrap() + } + + #[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(PyDictKeys::type_object(py)).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(PyDictValues::type_object(py)).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(PyDictItems::type_object(py)).unwrap()); + }) + } +} From 2d4b88334da2c2d22f7b44225987658af72af049 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 5 Jun 2022 16:05:40 +0100 Subject: [PATCH 06/34] Formatting --- pyo3-ffi/src/descrobject.rs | 2 +- src/types/mappingproxy.rs | 93 +++++++++++++++++++++++++++---------- src/types/mod.rs | 2 +- tests/test_mappingproxy.rs | 55 ++++++++++++++-------- 4 files changed, 106 insertions(+), 46 deletions(-) diff --git a/pyo3-ffi/src/descrobject.rs b/pyo3-ffi/src/descrobject.rs index 5d5ce2c35bf..9265f09e7e4 100644 --- a/pyo3-ffi/src/descrobject.rs +++ b/pyo3-ffi/src/descrobject.rs @@ -22,7 +22,7 @@ pub struct PyGetSetDef { #[derive(Copy, Clone, Debug)] pub struct PyDictProxyObject { pub ob_base: PyObject, - pub mapping: *mut PyObject + pub mapping: *mut PyObject, } // skipped non-limited wrapperfunc diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 1ee8d5c3326..3cbc429ffb0 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -1,18 +1,17 @@ // Copyright (c) 2017-present PyO3 Project and Contributors use super::PyMapping; -use crate::err::{PyResult}; -use crate::ffi::{PyDictProxy_Type, PyObject_TypeCheck}; +use crate::err::PyResult; use crate::ffi::addr_of_mut_shim; -use crate::types::{IntoPyDict, PyAny, PyDict, PyList, PyString, PyIterator, PySequence}; +use crate::ffi::{PyDictProxy_Type, PyObject_TypeCheck}; use crate::types::dict::PyDictItem; #[cfg(test)] -use crate::types::dict::{PyDictKeys, PyDictValues, PyDictItems, PyDictItem}; +use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; +use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence, PyString}; #[cfg(not(PyPy))] use crate::{ffi, AsPyPointer, PyObject, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; - #[inline] #[allow(non_snake_case)] unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { @@ -59,7 +58,10 @@ impl PyMappingProxy { /// This is equivalent to the Python expression `self.copy()`. pub fn copy(&self) -> PyResult<&PyDict> { unsafe { - let dict = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "copy").as_ptr()); + let dict = ffi::PyObject_CallMethodNoArgs( + self.as_ptr(), + PyString::new(self.py(), "copy").as_ptr(), + ); self.py().from_owned_ptr_or_err(dict) } } @@ -81,15 +83,16 @@ impl PyMappingProxy { PyAny::get_item(self, key).ok() } - /// Returns a list of mappingproxy keys. /// /// This is equivalent to the Python expression `list(mappingproxy.keys())`. pub fn keys(&self) -> &PyList { unsafe { - let keys = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "keys").as_ptr()); - self.py() - .from_owned_ptr::(keys).list().unwrap() + let keys = ffi::PyObject_CallMethodNoArgs( + self.as_ptr(), + PyString::new(self.py(), "keys").as_ptr(), + ); + self.py().from_owned_ptr::(keys).list().unwrap() } } @@ -98,9 +101,14 @@ impl PyMappingProxy { /// This is equivalent to the Python expression `list(mappingproxy.values())`. pub fn values(&self) -> &PyList { unsafe { - let values = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "values").as_ptr()); + let values = ffi::PyObject_CallMethodNoArgs( + self.as_ptr(), + PyString::new(self.py(), "values").as_ptr(), + ); self.py() - .from_owned_ptr::(values).list().unwrap() + .from_owned_ptr::(values) + .list() + .unwrap() } } @@ -109,9 +117,14 @@ impl PyMappingProxy { /// This is equivalent to the Python expression `list(mappingproxy.items())`. pub fn items(&self) -> &PyList { unsafe { - let items = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), PyString::new(self.py(), "items").as_ptr()); + let items = ffi::PyObject_CallMethodNoArgs( + self.as_ptr(), + PyString::new(self.py(), "items").as_ptr(), + ); self.py() - .from_owned_ptr::(items).list().unwrap() + .from_owned_ptr::(items) + .list() + .unwrap() } } @@ -129,7 +142,7 @@ impl PyMappingProxy { pub struct PyMappingProxyIterator<'py> { iterator: &'py PyIterator, - mappingproxy: &'py PyMappingProxy + mappingproxy: &'py PyMappingProxy, } impl<'py> Iterator for PyMappingProxyIterator<'py> { @@ -137,11 +150,10 @@ impl<'py> Iterator for PyMappingProxyIterator<'py> { #[inline] fn next(&mut self) -> Option { - self.iterator.next() + self.iterator + .next() .map(Result::unwrap) - .and_then( - |key| self.mappingproxy.get_item(key).map(|value| (key, value)) - ) + .and_then(|key| self.mappingproxy.get_item(key).map(|value| (key, value))) } } @@ -152,7 +164,7 @@ impl<'a> std::iter::IntoIterator for &'a PyMappingProxy { fn into_iter(self) -> Self::IntoIter { PyMappingProxyIterator { iterator: PyIterator::from_object(self.py(), self).unwrap(), - mappingproxy: self + mappingproxy: self, } } } @@ -187,7 +199,14 @@ mod tests { fn test_new() { Python::with_gil(|py| { let mappingproxy = [(7, 32)].into_py_mappingproxy(py).unwrap(); - assert_eq!(32, mappingproxy.get_item(7i32).unwrap().extract::().unwrap()); + assert_eq!( + 32, + mappingproxy + .get_item(7i32) + .unwrap() + .extract::() + .unwrap() + ); assert!(mappingproxy.get_item(8i32).is_none()); let map: HashMap = [(7, 32)].iter().cloned().collect(); assert_eq!(map, mappingproxy.extract().unwrap()); @@ -202,8 +221,22 @@ mod tests { Python::with_gil(|py| { let items = PyList::new(py, &vec![("a", 1), ("b", 2)]); let mappingproxy = PyMappingProxy::from_sequence(py, items.to_object(py)).unwrap(); - assert_eq!(1, mappingproxy.get_item("a").unwrap().extract::().unwrap()); - assert_eq!(2, mappingproxy.get_item("b").unwrap().extract::().unwrap()); + assert_eq!( + 1, + mappingproxy + .get_item("a") + .unwrap() + .extract::() + .unwrap() + ); + assert_eq!( + 2, + mappingproxy + .get_item("b") + .unwrap() + .extract::() + .unwrap() + ); let map: HashMap<&str, i32> = [("a", 1), ("b", 2)].iter().cloned().collect(); assert_eq!(map, mappingproxy.extract().unwrap()); let map: BTreeMap<&str, i32> = [("a", 1), ("b", 2)].iter().cloned().collect(); @@ -226,7 +259,10 @@ mod tests { let mappingproxy = [(7, 32)].into_py_mappingproxy(py).unwrap(); let new_dict = mappingproxy.copy().unwrap(); - assert_eq!(32, new_dict.get_item(7i32).unwrap().extract::().unwrap()); + assert_eq!( + 32, + new_dict.get_item(7i32).unwrap().extract::().unwrap() + ); assert!(new_dict.get_item(8i32).is_none()); }); } @@ -260,7 +296,14 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let mappingproxy = v.into_py_mappingproxy(py).unwrap(); - assert_eq!(32, mappingproxy.get_item(7i32).unwrap().extract::().unwrap()); + assert_eq!( + 32, + mappingproxy + .get_item(7i32) + .unwrap() + .extract::() + .unwrap() + ); assert!(mappingproxy.get_item(8i32).is_none()); }); } diff --git a/src/types/mod.rs b/src/types/mod.rs index f047cb14cf8..fe17f0c9a75 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -16,7 +16,6 @@ pub use self::datetime::{ PyTzInfoAccess, }; pub use self::dict::{IntoPyDict, PyDict}; -pub use self::mappingproxy::{IntoPyMappingProxy, PyMappingProxy}; pub use self::floatob::PyFloat; #[cfg(all(not(Py_LIMITED_API), not(PyPy)))] pub use self::frame::PyFrame; @@ -25,6 +24,7 @@ pub use self::function::{PyCFunction, PyFunction}; pub use self::iterator::PyIterator; pub use self::list::PyList; pub use self::mapping::PyMapping; +pub use self::mappingproxy::{IntoPyMappingProxy, PyMappingProxy}; pub use self::module::PyModule; pub use self::num::PyLong; pub use self::num::PyLong as PyInt; diff --git a/tests/test_mappingproxy.rs b/tests/test_mappingproxy.rs index 1ab76ba56ed..d82aace5092 100644 --- a/tests/test_mappingproxy.rs +++ b/tests/test_mappingproxy.rs @@ -1,12 +1,12 @@ use std::collections::HashMap; use pyo3::prelude::Python; -use pyo3::types::{IntoPyMappingProxy, PyString, PyInt}; +use pyo3::types::{IntoPyMappingProxy, PyInt, PyString}; const LEN: usize = 10_000_000; #[test] -fn get_value_from_mappingproxy_of_strings(){ +fn get_value_from_mappingproxy_of_strings() { let gil = Python::acquire_gil(); let py = gil.python(); @@ -19,18 +19,18 @@ fn get_value_from_mappingproxy_of_strings(){ assert_eq!( map.into_iter().collect::>(), - mappingproxy.iter().map( - |object| - ( - object.0.downcast::().unwrap().to_str().unwrap(), - object.1.downcast::().unwrap().to_str().unwrap() - ) - ).collect::>() + mappingproxy + .iter() + .map(|object| ( + object.0.downcast::().unwrap().to_str().unwrap(), + object.1.downcast::().unwrap().to_str().unwrap() + )) + .collect::>() ); } #[test] -fn get_value_from_mappingproxy_of_integers(){ +fn get_value_from_mappingproxy_of_integers() { let gil = Python::acquire_gil(); let py = gil.python(); @@ -38,17 +38,31 @@ fn get_value_from_mappingproxy_of_integers(){ let mappingproxy = items.to_vec().into_py_mappingproxy(py).unwrap(); assert_eq!( items, - mappingproxy.iter().map( - |object| - ( - object.0.downcast::().unwrap().extract::().unwrap(), - object.1.downcast::().unwrap().extract::().unwrap() - ) - ).collect::>() + mappingproxy + .iter() + .map(|object| ( + object + .0 + .downcast::() + .unwrap() + .extract::() + .unwrap(), + object + .1 + .downcast::() + .unwrap() + .extract::() + .unwrap() + )) + .collect::>() ); for index in 1..LEN { assert_eq!( - mappingproxy.get_item(index).unwrap().extract::().unwrap(), + mappingproxy + .get_item(index) + .unwrap() + .extract::() + .unwrap(), index - 1 ); } @@ -58,7 +72,10 @@ fn get_value_from_mappingproxy_of_integers(){ fn iter_mappingproxy_nosegv() { let gil = Python::acquire_gil(); let py = gil.python(); - let mappingproxy = (0..LEN as u64).map(|i| (i, i * 2)).into_py_mappingproxy(py).unwrap(); + let mappingproxy = (0..LEN as u64) + .map(|i| (i, i * 2)) + .into_py_mappingproxy(py) + .unwrap(); let mut sum = 0; for (k, _v) in mappingproxy.iter() { From 9783ae51dd47a49e01aaff7d361b88236845d158 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 5 Jun 2022 16:14:33 +0100 Subject: [PATCH 07/34] Remove unused imports present during testing --- src/types/mappingproxy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 3cbc429ffb0..680e52f7b80 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -192,7 +192,7 @@ mod tests { use super::*; #[cfg(not(PyPy))] use crate::{types::PyList, PyTypeInfo}; - use crate::{types::PyTuple, IntoPy, PyObject, PyTryFrom, Python, ToPyObject}; + use crate::{types::PyTuple, Python, ToPyObject}; use std::collections::{BTreeMap, HashMap}; #[test] From 11dfceb320087f058775764cd46285dd4d85c60e Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 5 Jun 2022 16:54:04 +0100 Subject: [PATCH 08/34] Replace calls to ffi::PyObject_CallMethodNoArgs with calls to PyAny::call_method0 --- src/types/mappingproxy.rs | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 680e52f7b80..967a535680a 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -7,7 +7,8 @@ use crate::ffi::{PyDictProxy_Type, PyObject_TypeCheck}; use crate::types::dict::PyDictItem; #[cfg(test)] use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; -use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence, PyString}; +use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence}; +use crate::PyErr; #[cfg(not(PyPy))] use crate::{ffi, AsPyPointer, PyObject, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; @@ -57,13 +58,8 @@ impl PyMappingProxy { /// /// This is equivalent to the Python expression `self.copy()`. pub fn copy(&self) -> PyResult<&PyDict> { - unsafe { - let dict = ffi::PyObject_CallMethodNoArgs( - self.as_ptr(), - PyString::new(self.py(), "copy").as_ptr(), - ); - self.py().from_owned_ptr_or_err(dict) - } + self.call_method0("copy") + .and_then(|object| object.downcast().map_err(PyErr::from)) } /// Checks if the mappingproxy is empty, i.e. `len(self) == 0`. @@ -88,11 +84,9 @@ impl PyMappingProxy { /// This is equivalent to the Python expression `list(mappingproxy.keys())`. pub fn keys(&self) -> &PyList { unsafe { - let keys = ffi::PyObject_CallMethodNoArgs( - self.as_ptr(), - PyString::new(self.py(), "keys").as_ptr(), - ); - self.py().from_owned_ptr::(keys).list().unwrap() + PySequence::try_from_unchecked(self.call_method0("keys").unwrap()) + .list() + .unwrap() } } @@ -101,12 +95,7 @@ impl PyMappingProxy { /// This is equivalent to the Python expression `list(mappingproxy.values())`. pub fn values(&self) -> &PyList { unsafe { - let values = ffi::PyObject_CallMethodNoArgs( - self.as_ptr(), - PyString::new(self.py(), "values").as_ptr(), - ); - self.py() - .from_owned_ptr::(values) + PySequence::try_from_unchecked(self.call_method0("values").unwrap()) .list() .unwrap() } @@ -117,12 +106,7 @@ impl PyMappingProxy { /// This is equivalent to the Python expression `list(mappingproxy.items())`. pub fn items(&self) -> &PyList { unsafe { - let items = ffi::PyObject_CallMethodNoArgs( - self.as_ptr(), - PyString::new(self.py(), "items").as_ptr(), - ); - self.py() - .from_owned_ptr::(items) + PySequence::try_from_unchecked(self.call_method0("items").unwrap()) .list() .unwrap() } From 7bb1f09dae2698922f97a8f84dc8c3d90e6dbfb8 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 5 Jun 2022 22:13:35 +0100 Subject: [PATCH 09/34] Add imports to PyPy build --- src/types/mappingproxy.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 967a535680a..c35a38f888d 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -2,21 +2,18 @@ use super::PyMapping; use crate::err::PyResult; -use crate::ffi::addr_of_mut_shim; -use crate::ffi::{PyDictProxy_Type, PyObject_TypeCheck}; use crate::types::dict::PyDictItem; #[cfg(test)] use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence}; use crate::PyErr; -#[cfg(not(PyPy))] use crate::{ffi, AsPyPointer, PyObject, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; #[inline] #[allow(non_snake_case)] unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { - PyObject_TypeCheck(object, addr_of_mut_shim!(PyDictProxy_Type)) + ffi::PyObject_TypeCheck(object, ffi::addr_of_mut_shim!(ffi::PyDictProxy_Type)) } /// Represents a Python `mappingproxy`. From f9b8d9e930bdacf87b266386eb3a7afa7f176cb8 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Mon, 6 Jun 2022 04:09:42 +0100 Subject: [PATCH 10/34] Remove unused import on PyPy builds --- src/types/mappingproxy.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index c35a38f888d..6804742e02e 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -7,7 +7,9 @@ use crate::types::dict::PyDictItem; use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence}; use crate::PyErr; -use crate::{ffi, AsPyPointer, PyObject, PyTryFrom, Python, ToPyObject}; +#[cfg(not(PyPy))] +use crate::PyObject; +use crate::{ffi, AsPyPointer, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; #[inline] From 4b9f02a430c081e2f0f46e6df47e1d5c534a11b3 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Mon, 6 Jun 2022 09:54:27 +0100 Subject: [PATCH 11/34] Remove unused imports from PyPy build --- src/types/mappingproxy.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 6804742e02e..d58f35d92a9 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -3,13 +3,13 @@ use super::PyMapping; use crate::err::PyResult; use crate::types::dict::PyDictItem; -#[cfg(test)] +#[cfg(all(test, not(PyPy)))] use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence}; use crate::PyErr; +use crate::{ffi, AsPyPointer, PyTryFrom, Python}; #[cfg(not(PyPy))] -use crate::PyObject; -use crate::{ffi, AsPyPointer, PyTryFrom, Python, ToPyObject}; +use crate::{PyObject, ToPyObject}; use std::os::raw::c_int; #[inline] From 83fe47a781d6f9a024121bcd150e2785b22ecdc8 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Mon, 6 Jun 2022 10:20:05 +0100 Subject: [PATCH 12/34] Add required imports to PyPy build --- src/types/mappingproxy.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index d58f35d92a9..23bb22d3c01 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -6,10 +6,9 @@ use crate::types::dict::PyDictItem; #[cfg(all(test, not(PyPy)))] use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence}; -use crate::PyErr; -use crate::{ffi, AsPyPointer, PyTryFrom, Python}; #[cfg(not(PyPy))] -use crate::{PyObject, ToPyObject}; +use crate::PyObject; +use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; #[inline] From 136d8fa9b7c20a412b272e39fa9788b6e8ff4480 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Mon, 6 Jun 2022 11:10:52 +0100 Subject: [PATCH 13/34] Move misplaced macro --- src/types/mappingproxy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 23bb22d3c01..4ac3e5cdd9d 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -6,8 +6,8 @@ use crate::types::dict::PyDictItem; #[cfg(all(test, not(PyPy)))] use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence}; -#[cfg(not(PyPy))] use crate::PyObject; +#[cfg(not(PyPy))] use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; From 69baaf84f4caad60746b0e55cb76cc7572d6f4d0 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Wed, 8 Jun 2022 16:36:16 +0100 Subject: [PATCH 14/34] Remove outdated comments from merge Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com> --- pyo3-ffi/src/descrobject.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pyo3-ffi/src/descrobject.rs b/pyo3-ffi/src/descrobject.rs index 9265f09e7e4..090d8002322 100644 --- a/pyo3-ffi/src/descrobject.rs +++ b/pyo3-ffi/src/descrobject.rs @@ -24,11 +24,6 @@ pub struct PyDictProxyObject { pub ob_base: PyObject, pub mapping: *mut PyObject, } - -// skipped non-limited wrapperfunc -// skipped non-limited wrapperfunc_kwds -// skipped non-limited struct wrapperbase -// skipped non-limited PyWrapperFlag_KEYWORDS impl Default for PyGetSetDef { fn default() -> PyGetSetDef { PyGetSetDef { From 00bac8e298611128f8900d1a86f2dc93f25c600d Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Wed, 8 Jun 2022 16:38:21 +0100 Subject: [PATCH 15/34] Remove #[inline] from private function Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com> --- src/types/mappingproxy.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 4ac3e5cdd9d..c39d00c5c9b 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -11,7 +11,6 @@ use crate::PyObject; use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; -#[inline] #[allow(non_snake_case)] unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { ffi::PyObject_TypeCheck(object, ffi::addr_of_mut_shim!(ffi::PyDictProxy_Type)) From 3bfb8113fd6021a21060d0dd6e8df918a5bf17b5 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Wed, 8 Jun 2022 16:39:07 +0100 Subject: [PATCH 16/34] Don't export PyDictProxyObject Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com> --- pyo3-ffi/src/descrobject.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pyo3-ffi/src/descrobject.rs b/pyo3-ffi/src/descrobject.rs index 090d8002322..63fe244fbcb 100644 --- a/pyo3-ffi/src/descrobject.rs +++ b/pyo3-ffi/src/descrobject.rs @@ -18,12 +18,6 @@ pub struct PyGetSetDef { pub closure: *mut c_void, } -#[repr(C)] -#[derive(Copy, Clone, Debug)] -pub struct PyDictProxyObject { - pub ob_base: PyObject, - pub mapping: *mut PyObject, -} impl Default for PyGetSetDef { fn default() -> PyGetSetDef { PyGetSetDef { From f4b5a051106447a03e05ad40911fa6061d3c4a84 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Wed, 8 Jun 2022 16:39:43 +0100 Subject: [PATCH 17/34] Don't rely on PyDictProxyObject Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com> --- src/types/mappingproxy.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index c39d00c5c9b..9b6ed088982 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -20,9 +20,8 @@ unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { #[repr(transparent)] pub struct PyMappingProxy(PyAny); -pyobject_native_type!( +pyobject_native_type_core!( PyMappingProxy, - ffi::PyDictProxyObject, ffi::PyDictProxy_Type, #checkfunction=PyDictProxy_Check ); From 7f98dd9f26a15c07c560a5204c7fdbda3d0ba366 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Thu, 9 Jun 2022 16:56:29 +0100 Subject: [PATCH 18/34] Don't export add_of_mut_shim macro Signed-off-by: Sam Ezeh --- pyo3-ffi/src/lib.rs | 1 - src/types/mappingproxy.rs | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pyo3-ffi/src/lib.rs b/pyo3-ffi/src/lib.rs index 221db56ec29..1e3ee85596e 100644 --- a/pyo3-ffi/src/lib.rs +++ b/pyo3-ffi/src/lib.rs @@ -264,7 +264,6 @@ macro_rules! opaque_struct { }; } -#[macro_export] macro_rules! addr_of_mut_shim { ($place:expr) => {{ #[cfg(addr_of)] diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 9b6ed088982..2c17f3884b9 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -11,9 +11,22 @@ use crate::PyObject; use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; +macro_rules! addr_of_mut_shim { + ($place:expr) => {{ + #[cfg(addr_of)] + { + ::std::ptr::addr_of_mut!($place) + } + #[cfg(not(addr_of))] + { + &mut $place as *mut _ + } + }}; +} + #[allow(non_snake_case)] unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { - ffi::PyObject_TypeCheck(object, ffi::addr_of_mut_shim!(ffi::PyDictProxy_Type)) + ffi::PyObject_TypeCheck(object, addr_of_mut_shim!(ffi::PyDictProxy_Type)) } /// Represents a Python `mappingproxy`. From 9bd2f440a5150176378bdcfbd6e68516aa020235 Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 12 Jun 2022 20:38:23 +0100 Subject: [PATCH 19/34] Replace instance of &mut with macro call --- src/pycell.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/pycell.rs b/src/pycell.rs index 928e7c3d1b1..97b29f1ef73 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -217,6 +217,19 @@ use std::marker::PhantomData; use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; +macro_rules! addr_of_mut_shim { + ($place:expr) => {{ + #[cfg(addr_of)] + { + ::std::ptr::addr_of_mut!($place) + } + #[cfg(not(addr_of))] + { + &mut $place as *mut _ + } + }}; +} + pub struct EmptySlot(()); pub struct BorrowChecker(Cell); @@ -1089,7 +1102,7 @@ where fn ensure_threadsafe(&self) {} unsafe fn tp_dealloc(slf: *mut ffi::PyObject, py: Python<'_>) { // For `#[pyclass]` types which inherit from PyAny, we can just call tp_free - if T::type_object_raw(py) == &mut PyBaseObject_Type { + if T::type_object_raw(py) == addr_of_mut_shim!(PyBaseObject_Type) { return get_tp_free(ffi::Py_TYPE(slf))(slf as _); } From c5e434d68eb3548e83dad378e3312139b426c65f Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 12 Jun 2022 21:18:56 +0100 Subject: [PATCH 20/34] Preserve error message when extracting dicts --- src/types/dict.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/types/dict.rs b/src/types/dict.rs index 8796092d037..3ffa7dc7a95 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -452,9 +452,12 @@ where } Ok(ret) } - Err(_) => { - let mappingproxy = ::try_from(ob)?; - mappingproxy.copy()?.extract() + Err(msg) => { + if let Ok(mappingproxy) = ::try_from(ob) { + mappingproxy.copy()?.extract() + } else { + Err(PyErr::from(msg)) + } } } } From 651a007d2886323b12079816af192e4a26563a9b Mon Sep 17 00:00:00 2001 From: Sam Ezeh Date: Sun, 12 Jun 2022 21:34:19 +0100 Subject: [PATCH 21/34] Add tests for PyDict::is_empty and PyMappingProxy::is_empty --- src/types/dict.rs | 10 ++++++++++ src/types/mappingproxy.rs | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/src/types/dict.rs b/src/types/dict.rs index 3ffa7dc7a95..9f7ad409408 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -676,6 +676,16 @@ mod tests { }); } + #[test] + fn test_is_empty() { + Python::with_gil(|py| { + let map: HashMap = HashMap::new(); + let object = map.to_object(py); + let dict = ::try_from(object.as_ref(py)).unwrap(); + assert!(dict.is_empty()); + }); + } + #[test] fn test_keys() { Python::with_gil(|py| { diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 2c17f3884b9..2b5c0e7840a 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -338,6 +338,15 @@ mod tests { }); } + #[test] + fn test_isempty() { + Python::with_gil(|py| { + let map: HashMap = HashMap::new(); + let mappingproxy = map.into_py_mappingproxy(py).unwrap(); + assert!(mappingproxy.is_empty()); + }); + } + #[test] fn test_keys() { Python::with_gil(|py| { From 1bcfe71688f65ca8963cd992457cde6c0580e402 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Sun, 15 Oct 2023 21:23:17 -0400 Subject: [PATCH 22/34] Fix build --- src/types/mappingproxy.rs | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 2b5c0e7840a..856f9e5ffba 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -11,22 +11,9 @@ use crate::PyObject; use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; -macro_rules! addr_of_mut_shim { - ($place:expr) => {{ - #[cfg(addr_of)] - { - ::std::ptr::addr_of_mut!($place) - } - #[cfg(not(addr_of))] - { - &mut $place as *mut _ - } - }}; -} - #[allow(non_snake_case)] unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { - ffi::PyObject_TypeCheck(object, addr_of_mut_shim!(ffi::PyDictProxy_Type)) + ffi::PyObject_TypeCheck(object, std::ptr::addr_of_mut!(ffi::PyDictProxy_Type)) } /// Represents a Python `mappingproxy`. @@ -35,7 +22,7 @@ pub struct PyMappingProxy(PyAny); pyobject_native_type_core!( PyMappingProxy, - ffi::PyDictProxy_Type, + pyobject_native_static_type_object!(ffi::PyDictProxy_Type), #checkfunction=PyDictProxy_Check ); @@ -94,7 +81,7 @@ impl PyMappingProxy { pub fn keys(&self) -> &PyList { unsafe { PySequence::try_from_unchecked(self.call_method0("keys").unwrap()) - .list() + .to_list() .unwrap() } } @@ -105,7 +92,7 @@ impl PyMappingProxy { pub fn values(&self) -> &PyList { unsafe { PySequence::try_from_unchecked(self.call_method0("values").unwrap()) - .list() + .to_list() .unwrap() } } @@ -116,7 +103,7 @@ impl PyMappingProxy { pub fn items(&self) -> &PyList { unsafe { PySequence::try_from_unchecked(self.call_method0("items").unwrap()) - .list() + .to_list() .unwrap() } } @@ -156,7 +143,7 @@ impl<'a> std::iter::IntoIterator for &'a PyMappingProxy { fn into_iter(self) -> Self::IntoIter { PyMappingProxyIterator { - iterator: PyIterator::from_object(self.py(), self).unwrap(), + iterator: PyIterator::from_object(self).unwrap(), mappingproxy: self, } } From 322b677324c257bee14216e381d76a7285455bab Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Sun, 15 Oct 2023 22:46:06 -0400 Subject: [PATCH 23/34] elements: &'py PyMapping --- src/types/dict.rs | 10 ----- src/types/mappingproxy.rs | 88 +++++++++------------------------------ 2 files changed, 20 insertions(+), 78 deletions(-) diff --git a/src/types/dict.rs b/src/types/dict.rs index 46b17b6c0c4..fa9bbe994f8 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -732,16 +732,6 @@ mod tests { }); } - #[test] - fn test_is_empty() { - Python::with_gil(|py| { - let map: HashMap = HashMap::new(); - let object = map.to_object(py); - let dict = ::try_from(object.as_ref(py)).unwrap(); - assert!(dict.is_empty()); - }); - } - #[test] fn test_keys() { Python::with_gil(|py| { diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 856f9e5ffba..5a0b9cfeed9 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -2,11 +2,10 @@ use super::PyMapping; use crate::err::PyResult; -use crate::types::dict::PyDictItem; +use crate::types::dict::{IntoPyDict, PyDictItem}; #[cfg(all(test, not(PyPy)))] use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; -use crate::types::{IntoPyDict, PyAny, PyDict, PyIterator, PyList, PySequence}; -use crate::PyObject; +use crate::types::{PyAny, PyDict, PyIterator, PyList, PySequence}; #[cfg(not(PyPy))] use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; @@ -28,28 +27,13 @@ pyobject_native_type_core!( impl PyMappingProxy { /// Creates a mappingproxy from an object. - pub fn new(py: Python<'_>, elements: impl IntoPyDict) -> PyResult<&'_ PyMappingProxy> { + pub fn new<'py>(py: Python<'py>, elements: &'py PyMapping) -> PyResult<&'py PyMappingProxy> { unsafe { - let dict = elements.into_py_dict(py); - let proxy = ffi::PyDictProxy_New(dict.as_ptr()); + let proxy = ffi::PyDictProxy_New(elements.as_ptr()); py.from_owned_ptr_or_err::(proxy) } } - /// Creates a new mappingproxy from the sequence given. - /// - /// The sequence must consist of `(PyObject, PyObject)`. - /// - /// Returns an error on invalid input. In the case of key collisions, - /// this keeps the last entry seen. - #[cfg(not(PyPy))] - pub fn from_sequence(py: Python<'_>, seq: PyObject) -> PyResult<&PyMappingProxy> { - unsafe { - let dict = py.from_owned_ptr::(PyDict::from_sequence(py, seq)?.as_ptr()); - py.from_owned_ptr_or_err(ffi::PyDictProxy_New(dict.as_ptr())) - } - } - /// Returns a new mappingproxy that contains the same key-value pairs as self. /// /// This is equivalent to the Python expression `self.copy()`. @@ -151,19 +135,20 @@ impl<'a> std::iter::IntoIterator for &'a PyMappingProxy { /// Conversion trait that allows a sequence of tuples to be converted into `PyMappingProxy` /// Primary use case for this trait is `call` and `call_method` methods as keywords argument. -pub trait IntoPyMappingProxy { +pub trait IntoPyMappingProxy<'py> { /// Converts self into a `PyMappingProxy` object pointer. Whether pointer owned or borrowed /// depends on implementation. - fn into_py_mappingproxy(self, py: Python<'_>) -> PyResult<&PyMappingProxy>; + fn into_py_mappingproxy(self, py: Python<'py>) -> PyResult<&'py PyMappingProxy>; } -impl IntoPyMappingProxy for I +impl<'py, T, I> IntoPyMappingProxy<'py> for I where T: PyDictItem, I: IntoIterator, { - fn into_py_mappingproxy(self, py: Python<'_>) -> PyResult<&PyMappingProxy> { - PyMappingProxy::new(py, self) + fn into_py_mappingproxy(self, py: Python<'py>) -> PyResult<&'py PyMappingProxy> { + let dict = self.into_py_dict(py); + PyMappingProxy::new(py, dict.as_mapping()) } } @@ -171,8 +156,7 @@ where mod tests { use super::*; #[cfg(not(PyPy))] - use crate::{types::PyList, PyTypeInfo}; - use crate::{types::PyTuple, Python, ToPyObject}; + use crate::{types::PyTuple, PyTypeInfo, Python}; use std::collections::{BTreeMap, HashMap}; #[test] @@ -195,44 +179,6 @@ mod tests { }); } - #[test] - #[cfg(not(PyPy))] - fn test_from_sequence() { - Python::with_gil(|py| { - let items = PyList::new(py, &vec![("a", 1), ("b", 2)]); - let mappingproxy = PyMappingProxy::from_sequence(py, items.to_object(py)).unwrap(); - assert_eq!( - 1, - mappingproxy - .get_item("a") - .unwrap() - .extract::() - .unwrap() - ); - assert_eq!( - 2, - mappingproxy - .get_item("b") - .unwrap() - .extract::() - .unwrap() - ); - let map: HashMap<&str, i32> = [("a", 1), ("b", 2)].iter().cloned().collect(); - assert_eq!(map, mappingproxy.extract().unwrap()); - let map: BTreeMap<&str, i32> = [("a", 1), ("b", 2)].iter().cloned().collect(); - assert_eq!(map, mappingproxy.extract().unwrap()); - }); - } - - #[test] - #[cfg(not(PyPy))] - fn test_from_sequence_err() { - Python::with_gil(|py| { - let items = PyList::new(py, &vec!["a", "b"]); - assert!(PyMappingProxy::from_sequence(py, items.to_object(py)).is_err()); - }); - } - #[test] fn test_copy() { Python::with_gil(|py| { @@ -241,9 +187,15 @@ mod tests { let new_dict = mappingproxy.copy().unwrap(); assert_eq!( 32, - new_dict.get_item(7i32).unwrap().extract::().unwrap() + new_dict + .get_item(7i32) + .unwrap() + .unwrap() + .extract::() + .unwrap() ); - assert!(new_dict.get_item(8i32).is_none()); + assert!(new_dict.get_item(8i32).unwrap().is_none()); + assert!(new_dict.get_item(8i32).unwrap().is_none()); }); } @@ -316,7 +268,7 @@ mod tests { let mut key_sum = 0; let mut value_sum = 0; for el in mappingproxy.items().iter() { - let tuple = el.cast_as::().unwrap(); + let tuple = el.downcast::().unwrap(); key_sum += tuple.get_item(0).unwrap().extract::().unwrap(); value_sum += tuple.get_item(1).unwrap().extract::().unwrap(); } From f7e4436e5d6636909087ffa374ee0344e1823f5a Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Sun, 15 Oct 2023 23:06:41 -0400 Subject: [PATCH 24/34] Conversions --- src/conversions/hashbrown.rs | 28 +++++++++++++++---- src/conversions/indexmap.rs | 28 +++++++++++++++---- src/conversions/std/map.rs | 54 ++++++++++++++++++++++++++++-------- 3 files changed, 89 insertions(+), 21 deletions(-) diff --git a/src/conversions/hashbrown.rs b/src/conversions/hashbrown.rs index 6e20db39165..63446ca4298 100644 --- a/src/conversions/hashbrown.rs +++ b/src/conversions/hashbrown.rs @@ -55,12 +55,30 @@ where S: hash::BuildHasher + Default, { fn extract(ob: &'source PyAny) -> Result { - let dict: &PyDict = ob.downcast()?; - let mut ret = hashbrown::HashMap::with_capacity_and_hasher(dict.len(), S::default()); - for (k, v) in dict { - ret.insert(K::extract(k)?, V::extract(v)?); + match ob.downcast::() { + Ok(dict) => { + let mut ret = + hashbrown::HashMap::with_capacity_and_hasher(dict.len(), S::default()); + for (k, v) in dict { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } + Err(msg) => { + if let Ok(mappingproxy) = ob.downcast::() { + let mut ret = hashbrown::HashMap::with_capacity_and_hasher( + mappingproxy.len(), + S::default(), + ); + for (k, v) in mappingproxy { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } else { + Err(PyErr::from(msg)) + } + } } - Ok(ret) } } diff --git a/src/conversions/indexmap.rs b/src/conversions/indexmap.rs index 7c7303e6d83..ec3b2f6eea5 100644 --- a/src/conversions/indexmap.rs +++ b/src/conversions/indexmap.rs @@ -123,12 +123,30 @@ where S: hash::BuildHasher + Default, { fn extract(ob: &'source PyAny) -> Result { - let dict: &PyDict = ob.downcast()?; - let mut ret = indexmap::IndexMap::with_capacity_and_hasher(dict.len(), S::default()); - for (k, v) in dict { - ret.insert(K::extract(k)?, V::extract(v)?); + match ob.downcast::() { + Ok(dict) => { + let mut ret = + indexmap::IndexMap::with_capacity_and_hasher(dict.len(), S::default()); + for (k, v) in dict { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } + Err(msg) => { + if let Ok(mappingproxy) = ob.downcast::() { + let mut ret = indexmap::IndexMap::with_capacity_and_hasher( + mappingproxy.len(), + S::default(), + ); + for (k, v) in mappingproxy { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } else { + Err(PyErr::from(msg)) + } + } } - Ok(ret) } } diff --git a/src/conversions/std/map.rs b/src/conversions/std/map.rs index a53b0ce9222..7cec38418fc 100644 --- a/src/conversions/std/map.rs +++ b/src/conversions/std/map.rs @@ -3,7 +3,7 @@ use std::{cmp, collections, hash}; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::{ - types::{IntoPyDict, PyDict}, + types::{IntoPyDict, PyDict, PyMappingProxy}, FromPyObject, IntoPy, PyAny, PyErr, PyObject, Python, ToPyObject, }; @@ -72,12 +72,30 @@ where S: hash::BuildHasher + Default, { fn extract(ob: &'source PyAny) -> Result { - let dict: &PyDict = ob.downcast()?; - let mut ret = collections::HashMap::with_capacity_and_hasher(dict.len(), S::default()); - for (k, v) in dict { - ret.insert(K::extract(k)?, V::extract(v)?); + match ob.downcast::() { + Ok(dict) => { + let mut ret = + collections::HashMap::with_capacity_and_hasher(dict.len(), S::default()); + for (k, v) in dict { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } + Err(msg) => { + if let Ok(mappingproxy) = ob.downcast::() { + let mut ret = collections::HashMap::with_capacity_and_hasher( + mappingproxy.len()?, + S::default(), + ); + for (k, v) in mappingproxy { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } else { + Err(PyErr::from(msg)) + } + } } - Ok(ret) } #[cfg(feature = "experimental-inspect")] @@ -92,12 +110,26 @@ where V: FromPyObject<'source>, { fn extract(ob: &'source PyAny) -> Result { - let dict: &PyDict = ob.downcast()?; - let mut ret = collections::BTreeMap::new(); - for (k, v) in dict { - ret.insert(K::extract(k)?, V::extract(v)?); + match ob.downcast::() { + Ok(dict) => { + let mut ret = collections::BTreeMap::new(); + for (k, v) in dict { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } + Err(msg) => { + if let Ok(mappingproxy) = ob.downcast::() { + let mut ret = collections::BTreeMap::new(); + for (k, v) in mappingproxy { + ret.insert(K::extract(k)?, V::extract(v)?); + } + Ok(ret) + } else { + Err(PyErr::from(msg)) + } + } } - Ok(ret) } #[cfg(feature = "experimental-inspect")] From 10c007c2318e3af69de0be903a323ff139acc04e Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Sun, 15 Oct 2023 23:22:46 -0400 Subject: [PATCH 25/34] Move tests --- src/types/mappingproxy.rs | 83 +++++++++++++++++++++++++++++++++++- tests/test_mappingproxy.rs | 86 -------------------------------------- 2 files changed, 82 insertions(+), 87 deletions(-) delete mode 100644 tests/test_mappingproxy.rs diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 5a0b9cfeed9..153d9add2c4 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -155,8 +155,9 @@ where #[cfg(test)] mod tests { use super::*; + use crate::types::{PyInt, PyString, PyTuple}; #[cfg(not(PyPy))] - use crate::{types::PyTuple, PyTypeInfo, Python}; + use crate::{PyTypeInfo, Python}; use std::collections::{BTreeMap, HashMap}; #[test] @@ -519,4 +520,84 @@ mod tests { assert!(items.is_instance(PyDictItems::type_object(py)).unwrap()); }) } + + #[test] + fn get_value_from_mappingproxy_of_strings() { + Python::with_gil(|py: Python<'_>| { + let mut map = HashMap::new(); + map.insert("first key", "first value"); + map.insert("second key", "second value"); + map.insert("third key", "third value"); + + let mappingproxy = map.iter().into_py_mappingproxy(py).unwrap(); + + assert_eq!( + map.into_iter().collect::>(), + mappingproxy + .iter() + .map(|object| ( + object.0.downcast::().unwrap().to_str().unwrap(), + object.1.downcast::().unwrap().to_str().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 mappingproxy = items.to_vec().into_py_mappingproxy(py).unwrap(); + assert_eq!( + items, + mappingproxy + .iter() + .map(|object| ( + object + .0 + .downcast::() + .unwrap() + .extract::() + .unwrap(), + object + .1 + .downcast::() + .unwrap() + .extract::() + .unwrap() + )) + .collect::>() + ); + for index in 1..LEN { + assert_eq!( + mappingproxy + .get_item(index) + .unwrap() + .extract::() + .unwrap(), + index - 1 + ); + } + }) + } + + #[test] + fn iter_mappingproxy_nosegv() { + Python::with_gil(|py| { + const LEN: usize = 10_000_000; + let mappingproxy = (0..LEN as u64) + .map(|i| (i, i * 2)) + .into_py_mappingproxy(py) + .unwrap(); + + let mut sum = 0; + for (k, _v) in mappingproxy.iter() { + let i: u64 = k.extract().unwrap(); + sum += i; + } + assert_eq!(sum, 49_999_995_000_000); + }) + } } diff --git a/tests/test_mappingproxy.rs b/tests/test_mappingproxy.rs deleted file mode 100644 index d82aace5092..00000000000 --- a/tests/test_mappingproxy.rs +++ /dev/null @@ -1,86 +0,0 @@ -use std::collections::HashMap; - -use pyo3::prelude::Python; -use pyo3::types::{IntoPyMappingProxy, PyInt, PyString}; - -const LEN: usize = 10_000_000; - -#[test] -fn get_value_from_mappingproxy_of_strings() { - let gil = Python::acquire_gil(); - let py = gil.python(); - - let mut map = HashMap::new(); - map.insert("first key", "first value"); - map.insert("second key", "second value"); - map.insert("third key", "third value"); - - let mappingproxy = map.iter().into_py_mappingproxy(py).unwrap(); - - assert_eq!( - map.into_iter().collect::>(), - mappingproxy - .iter() - .map(|object| ( - object.0.downcast::().unwrap().to_str().unwrap(), - object.1.downcast::().unwrap().to_str().unwrap() - )) - .collect::>() - ); -} - -#[test] -fn get_value_from_mappingproxy_of_integers() { - let gil = Python::acquire_gil(); - let py = gil.python(); - - let items: Vec<(usize, usize)> = (1..LEN).map(|i| (i, i - 1)).collect(); - let mappingproxy = items.to_vec().into_py_mappingproxy(py).unwrap(); - assert_eq!( - items, - mappingproxy - .iter() - .map(|object| ( - object - .0 - .downcast::() - .unwrap() - .extract::() - .unwrap(), - object - .1 - .downcast::() - .unwrap() - .extract::() - .unwrap() - )) - .collect::>() - ); - for index in 1..LEN { - assert_eq!( - mappingproxy - .get_item(index) - .unwrap() - .extract::() - .unwrap(), - index - 1 - ); - } -} - -#[test] -fn iter_mappingproxy_nosegv() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let mappingproxy = (0..LEN as u64) - .map(|i| (i, i * 2)) - .into_py_mappingproxy(py) - .unwrap(); - - let mut sum = 0; - for (k, _v) in mappingproxy.iter() { - let i: u64 = k.extract().unwrap(); - sum += i; - } - assert_eq!(sum, 49_999_995_000_000); -} From 8336961cfbfc9b314d096bafd210e6831b67e422 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Sun, 15 Oct 2023 23:27:48 -0400 Subject: [PATCH 26/34] Return PyMapping on copy --- src/types/mappingproxy.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 153d9add2c4..90ace841542 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -5,7 +5,7 @@ use crate::err::PyResult; use crate::types::dict::{IntoPyDict, PyDictItem}; #[cfg(all(test, not(PyPy)))] use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; -use crate::types::{PyAny, PyDict, PyIterator, PyList, PySequence}; +use crate::types::{PyAny, PyIterator, PyList, PySequence}; #[cfg(not(PyPy))] use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; use std::os::raw::c_int; @@ -37,9 +37,9 @@ impl PyMappingProxy { /// Returns a new mappingproxy that contains the same key-value pairs as self. /// /// This is equivalent to the Python expression `self.copy()`. - pub fn copy(&self) -> PyResult<&PyDict> { + pub fn copy(&self) -> PyResult<&PyMapping> { self.call_method0("copy") - .and_then(|object| object.downcast().map_err(PyErr::from)) + .and_then(|object| object.downcast::().map_err(PyErr::from)) } /// Checks if the mappingproxy is empty, i.e. `len(self) == 0`. @@ -155,7 +155,10 @@ where #[cfg(test)] mod tests { use super::*; - use crate::types::{PyInt, PyString, PyTuple}; + use crate::{ + exceptions::PyKeyError, + types::{PyInt, PyString, PyTuple}, + }; #[cfg(not(PyPy))] use crate::{PyTypeInfo, Python}; use std::collections::{BTreeMap, HashMap}; @@ -188,15 +191,12 @@ mod tests { let new_dict = mappingproxy.copy().unwrap(); assert_eq!( 32, - new_dict - .get_item(7i32) - .unwrap() - .unwrap() - .extract::() - .unwrap() + new_dict.get_item(7i32).unwrap().extract::().unwrap() ); - assert!(new_dict.get_item(8i32).unwrap().is_none()); - assert!(new_dict.get_item(8i32).unwrap().is_none()); + assert!(new_dict + .get_item(8i32) + .unwrap_err() + .is_instance_of::(py)); }); } From b47bea9911c5f631a918c33477bd950505317976 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Sun, 15 Oct 2023 23:43:35 -0400 Subject: [PATCH 27/34] newfragments --- newsfragments/3521.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3521.added.md diff --git a/newsfragments/3521.added.md b/newsfragments/3521.added.md new file mode 100644 index 00000000000..092f00f8a8e --- /dev/null +++ b/newsfragments/3521.added.md @@ -0,0 +1 @@ +Add `PyMappingProxy` type. From 54fdc00e8f45a5b19fd5308d8a0e9e8d63b76cd8 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Sun, 15 Oct 2023 23:49:59 -0400 Subject: [PATCH 28/34] unwrap_or_default --- src/conversions/hashbrown.rs | 2 +- src/conversions/indexmap.rs | 2 +- src/conversions/std/map.rs | 2 +- src/types/mappingproxy.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conversions/hashbrown.rs b/src/conversions/hashbrown.rs index 63446ca4298..06786693ecf 100644 --- a/src/conversions/hashbrown.rs +++ b/src/conversions/hashbrown.rs @@ -67,7 +67,7 @@ where Err(msg) => { if let Ok(mappingproxy) = ob.downcast::() { let mut ret = hashbrown::HashMap::with_capacity_and_hasher( - mappingproxy.len(), + mappingproxy.len().unwrap_or_default(), S::default(), ); for (k, v) in mappingproxy { diff --git a/src/conversions/indexmap.rs b/src/conversions/indexmap.rs index ec3b2f6eea5..c424e9b111d 100644 --- a/src/conversions/indexmap.rs +++ b/src/conversions/indexmap.rs @@ -135,7 +135,7 @@ where Err(msg) => { if let Ok(mappingproxy) = ob.downcast::() { let mut ret = indexmap::IndexMap::with_capacity_and_hasher( - mappingproxy.len(), + mappingproxy.len().unwrap_or_default(), S::default(), ); for (k, v) in mappingproxy { diff --git a/src/conversions/std/map.rs b/src/conversions/std/map.rs index 7cec38418fc..55a0618068c 100644 --- a/src/conversions/std/map.rs +++ b/src/conversions/std/map.rs @@ -84,7 +84,7 @@ where Err(msg) => { if let Ok(mappingproxy) = ob.downcast::() { let mut ret = collections::HashMap::with_capacity_and_hasher( - mappingproxy.len()?, + mappingproxy.len().unwrap_or_default(), S::default(), ); for (k, v) in mappingproxy { diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 90ace841542..4286c3c80a4 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -44,7 +44,7 @@ impl PyMappingProxy { /// Checks if the mappingproxy is empty, i.e. `len(self) == 0`. pub fn is_empty(&self) -> bool { - self.len().unwrap() == 0 + self.len().unwrap_or_default() == 0 } /// Gets an item from the mappingproxy. From 5ae7d644336fa565d235cc87a06c13bb09d54e91 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 16 Oct 2023 00:03:28 -0400 Subject: [PATCH 29/34] Fix --- src/conversions/hashbrown.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conversions/hashbrown.rs b/src/conversions/hashbrown.rs index 06786693ecf..760dcaec970 100644 --- a/src/conversions/hashbrown.rs +++ b/src/conversions/hashbrown.rs @@ -18,7 +18,7 @@ //! The required hashbrown version may vary based on the version of PyO3. use crate::{ types::set::new_from_iter, - types::{IntoPyDict, PyDict, PySet}, + types::{IntoPyDict, PyDict, PyMappingProxy, PySet}, FromPyObject, IntoPy, PyAny, PyErr, PyObject, PyResult, Python, ToPyObject, }; use std::{cmp, hash}; From 38671adf13b05e4c1662b318e9b909438d4e721b Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 16 Oct 2023 00:14:29 -0400 Subject: [PATCH 30/34] Test hashbrown + indexmap conversions --- src/conversions/hashbrown.rs | 17 +++++++++++++++++ src/conversions/indexmap.rs | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/conversions/hashbrown.rs b/src/conversions/hashbrown.rs index 760dcaec970..bb422320374 100644 --- a/src/conversions/hashbrown.rs +++ b/src/conversions/hashbrown.rs @@ -119,6 +119,7 @@ where #[cfg(test)] mod tests { use super::*; + use crate::types::IntoPyMappingProxy; #[test] fn test_hashbrown_hashmap_to_python() { @@ -204,4 +205,20 @@ mod tests { assert_eq!(hs, hso.extract(py).unwrap()); }); } + + #[test] + fn test_hashbrown_hashmap_into_mapping_proxy() { + Python::with_gil(|py| { + let mut map = hashbrown::HashMap::::new(); + map.insert(1, 1); + + let mappingproxy = map.into_py_mappingproxy(py).unwrap(); + + assert_eq!(mappingproxy.len().unwrap(), 1); + assert_eq!( + mappingproxy.get_item(1).unwrap().extract::().unwrap(), + 1 + ); + }); + } } diff --git a/src/conversions/indexmap.rs b/src/conversions/indexmap.rs index c424e9b111d..f0a4b321227 100644 --- a/src/conversions/indexmap.rs +++ b/src/conversions/indexmap.rs @@ -254,4 +254,20 @@ mod test_indexmap { } }); } + + #[test] + fn test_indexmap_indexmap_into_mappingproxy() { + Python::with_gil(|py| { + let mut map = indexmap::IndexMap::::new(); + map.insert(1, 1); + + let mappingproxy = map.into_py_mappingproxy(py).unwrap(); + + assert_eq!(mappingproxy.len().unwrap(), 1); + assert_eq!( + mappingproxy.get_item(1).unwrap().extract::().unwrap(), + 1 + ); + }); + } } From e949a7ec9de36cb31220b173f747c974b704d155 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 16 Oct 2023 00:48:30 -0400 Subject: [PATCH 31/34] Clippy and cargo check --- pyo3-ffi/src/descrobject.rs | 7 ++++++- src/types/mappingproxy.rs | 27 +++++++++------------------ 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/pyo3-ffi/src/descrobject.rs b/pyo3-ffi/src/descrobject.rs index f4a5ce00bf6..dd8e1784726 100644 --- a/pyo3-ffi/src/descrobject.rs +++ b/pyo3-ffi/src/descrobject.rs @@ -1,6 +1,6 @@ use crate::methodobject::PyMethodDef; use crate::object::{PyObject, PyTypeObject}; -use crate::Py_ssize_t; +use crate::{PyObject_TypeCheck, Py_ssize_t}; use std::os::raw::{c_char, c_int, c_void}; use std::ptr; @@ -68,6 +68,11 @@ extern "C" { pub fn PyWrapper_New(arg1: *mut PyObject, arg2: *mut PyObject) -> *mut PyObject; } +#[inline] +pub unsafe fn PyDictProxy_Check(op: *mut PyObject) -> c_int { + PyObject_TypeCheck(op, std::ptr::addr_of_mut!(PyDictProxy_Type)) +} + /// Represents the [PyMemberDef](https://docs.python.org/3/c-api/structures.html#c.PyMemberDef) /// structure. /// diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 4286c3c80a4..4da06c4c3bb 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -3,17 +3,8 @@ use super::PyMapping; use crate::err::PyResult; use crate::types::dict::{IntoPyDict, PyDictItem}; -#[cfg(all(test, not(PyPy)))] -use crate::types::dict::{PyDictItems, PyDictKeys, PyDictValues}; use crate::types::{PyAny, PyIterator, PyList, PySequence}; -#[cfg(not(PyPy))] use crate::{ffi, AsPyPointer, PyErr, PyTryFrom, Python, ToPyObject}; -use std::os::raw::c_int; - -#[allow(non_snake_case)] -unsafe fn PyDictProxy_Check(object: *mut crate::ffi::PyObject) -> c_int { - ffi::PyObject_TypeCheck(object, std::ptr::addr_of_mut!(ffi::PyDictProxy_Type)) -} /// Represents a Python `mappingproxy`. #[repr(transparent)] @@ -22,7 +13,7 @@ pub struct PyMappingProxy(PyAny); pyobject_native_type_core!( PyMappingProxy, pyobject_native_static_type_object!(ffi::PyDictProxy_Type), - #checkfunction=PyDictProxy_Check + #checkfunction=ffi::PyDictProxy_Check ); impl PyMappingProxy { @@ -155,12 +146,12 @@ where #[cfg(test)] mod tests { use super::*; + use crate::type_object::PyTypeInfo; + use crate::Python; use crate::{ exceptions::PyKeyError, - types::{PyInt, PyString, PyTuple}, + types::{PyDictItems, PyDictKeys, PyDictValues, PyInt, PyString, PyTuple}, }; - #[cfg(not(PyPy))] - use crate::{PyTypeInfo, Python}; use std::collections::{BTreeMap, HashMap}; #[test] @@ -268,7 +259,7 @@ mod tests { // 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 el in mappingproxy.items().iter() { + for el in mappingproxy.items() { let tuple = el.downcast::().unwrap(); key_sum += tuple.get_item(0).unwrap().extract::().unwrap(); value_sum += tuple.get_item(1).unwrap().extract::().unwrap(); @@ -297,7 +288,7 @@ mod tests { let mappingproxy = v.into_py_mappingproxy(py).unwrap(); // 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().iter() { + for el in mappingproxy.keys() { key_sum += el.extract::().unwrap(); } assert_eq!(7 + 8 + 9, key_sum); @@ -314,7 +305,7 @@ mod tests { let mappingproxy = v.into_py_mappingproxy(py).unwrap(); // 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().iter() { + for el in mappingproxy.values() { values_sum += el.extract::().unwrap(); } assert_eq!(32 + 42 + 123, values_sum); @@ -331,7 +322,7 @@ mod tests { let mappingproxy = v.into_py_mappingproxy(py).unwrap(); let mut key_sum = 0; let mut value_sum = 0; - for (key, value) in mappingproxy.iter() { + for (key, value) in mappingproxy { key_sum += key.extract::().unwrap(); value_sum += value.extract::().unwrap(); } @@ -593,7 +584,7 @@ mod tests { .unwrap(); let mut sum = 0; - for (k, _v) in mappingproxy.iter() { + for (k, _v) in mappingproxy { let i: u64 = k.extract().unwrap(); sum += i; } From 16df9bf8ccc24453f871359b2b65761269b02d4e Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 16 Oct 2023 09:38:14 -0400 Subject: [PATCH 32/34] Clean up test imports --- src/types/mappingproxy.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 4da06c4c3bb..8766da2a0b6 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -146,11 +146,12 @@ where #[cfg(test)] mod tests { use super::*; - use crate::type_object::PyTypeInfo; use crate::Python; use crate::{ exceptions::PyKeyError, - types::{PyDictItems, PyDictKeys, PyDictValues, PyInt, PyString, PyTuple}, + type_object::PyTypeInfo, + types::dict::{PyDictItems, PyDictKeys, PyDictValues}, + types::{PyInt, PyString, PyTuple}, }; use std::collections::{BTreeMap, HashMap}; From f66c610cf346b2988129d640daa6ba90aa07608f Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 16 Oct 2023 09:54:32 -0400 Subject: [PATCH 33/34] Test hashbrown/indexmap extract conversions --- src/conversions/hashbrown.rs | 8 +++++++- src/conversions/indexmap.rs | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/conversions/hashbrown.rs b/src/conversions/hashbrown.rs index bb422320374..df82219ed6e 100644 --- a/src/conversions/hashbrown.rs +++ b/src/conversions/hashbrown.rs @@ -212,13 +212,19 @@ mod tests { let mut map = hashbrown::HashMap::::new(); map.insert(1, 1); - let mappingproxy = map.into_py_mappingproxy(py).unwrap(); + let mappingproxy = map.clone().into_py_mappingproxy(py).unwrap(); assert_eq!(mappingproxy.len().unwrap(), 1); assert_eq!( mappingproxy.get_item(1).unwrap().extract::().unwrap(), 1 ); + assert_eq!( + map, + mappingproxy + .extract::>() + .unwrap() + ); }); } } diff --git a/src/conversions/indexmap.rs b/src/conversions/indexmap.rs index f0a4b321227..aabb76c28bd 100644 --- a/src/conversions/indexmap.rs +++ b/src/conversions/indexmap.rs @@ -261,13 +261,19 @@ mod test_indexmap { let mut map = indexmap::IndexMap::::new(); map.insert(1, 1); - let mappingproxy = map.into_py_mappingproxy(py).unwrap(); + let mappingproxy = map.clone().into_py_mappingproxy(py).unwrap(); assert_eq!(mappingproxy.len().unwrap(), 1); assert_eq!( mappingproxy.get_item(1).unwrap().extract::().unwrap(), 1 ); + assert_eq!( + map, + mappingproxy + .extract::>() + .unwrap() + ); }); } } From 5005fc3a3c72c5afbbf42c9751ae6d5a0fc08ab4 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 16 Oct 2023 09:57:40 -0400 Subject: [PATCH 34/34] #[cfg(not(PyPy))] --- src/types/mappingproxy.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/types/mappingproxy.rs b/src/types/mappingproxy.rs index 8766da2a0b6..bb899cee0f7 100644 --- a/src/types/mappingproxy.rs +++ b/src/types/mappingproxy.rs @@ -146,11 +146,13 @@ where #[cfg(test)] mod tests { use super::*; + #[cfg(not(PyPy))] + use crate::type_object::PyTypeInfo; + #[cfg(not(PyPy))] + use crate::types::dict::*; use crate::Python; use crate::{ exceptions::PyKeyError, - type_object::PyTypeInfo, - types::dict::{PyDictItems, PyDictKeys, PyDictValues}, types::{PyInt, PyString, PyTuple}, }; use std::collections::{BTreeMap, HashMap};