Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adds the Bound<'_, PyMappingProxy> type #4644

Merged
merged 14 commits into from
Oct 29, 2024
Merged
1 change: 1 addition & 0 deletions newsfragments/4644.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
New `PyMappingProxy` struct corresponing to the `mappingproxy` class in Python.
7 changes: 6 additions & 1 deletion pyo3-ffi/src/descrobject.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function doesn't exist in https://github.com/python/cpython/blob/3.13/Include/descrobject.h (indeed I couldn't find it anywhere). So we shouldn't add this.

Suggested change
#[inline]
pub unsafe fn PyDictProxy_Check(op: *mut PyObject) -> c_int {
PyObject_TypeCheck(op, std::ptr::addr_of_mut!(PyDictProxy_Type))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed!


/// Represents the [PyMemberDef](https://docs.python.org/3/c-api/structures.html#c.PyMemberDef)
/// structure.
///
Expand Down
34 changes: 28 additions & 6 deletions src/conversions/std/map.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to considering the conversion changes in this file, but these changes might break existing users' code silently. I'm unsure both if it's worth the churn and also what the performance cost might be.

If we move ahead with these we should also include the corresponding changes to the index map and hash brown conversions.

At a minimum I would at like to see these changes split off into a separate PR, although I will be honest that there is a chance we might decide not to have these. Was it critical for your use case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went ahead and removed the conversion changes. It isn't necessary for me, I just had it in there because of the previous PR.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ use crate::ToPyObject;
use crate::{
conversion::IntoPyObject,
instance::Bound,
types::{any::PyAnyMethods, dict::PyDictMethods, PyDict},
types::{
any::PyAnyMethods, dict::PyDictMethods, mappingproxy::PyMappingProxyMethods, PyDict,
PyMappingProxy,
},
FromPyObject, IntoPy, PyAny, PyErr, PyObject, Python,
};

Expand Down Expand Up @@ -162,9 +165,19 @@ where
S: hash::BuildHasher + Default,
{
fn extract_bound(ob: &Bound<'py, PyAny>) -> Result<Self, PyErr> {
let dict = ob.downcast::<PyDict>()?;
let mut ret = collections::HashMap::with_capacity_and_hasher(dict.len(), S::default());
for (k, v) in dict {
if let Ok(dict) = ob.downcast::<PyDict>() {
let mut ret = collections::HashMap::with_capacity_and_hasher(dict.len(), S::default());
for (k, v) in dict {
ret.insert(k.extract()?, v.extract()?);
}
return Ok(ret);
}

let mappingproxy = ob.downcast::<PyMappingProxy>()?;
let mut ret =
collections::HashMap::with_capacity_and_hasher(mappingproxy.len()?, S::default());
for res in mappingproxy.try_iter()? {
let (k, v) = res?;
ret.insert(k.extract()?, v.extract()?);
}
Ok(ret)
Expand All @@ -182,9 +195,18 @@ where
V: FromPyObject<'py>,
{
fn extract_bound(ob: &Bound<'py, PyAny>) -> Result<Self, PyErr> {
let dict = ob.downcast::<PyDict>()?;
if let Ok(dict) = ob.downcast::<PyDict>() {
let mut ret = collections::BTreeMap::new();
for (k, v) in dict {
ret.insert(k.extract()?, v.extract()?);
}
return Ok(ret);
}

let mappingproxy = ob.downcast::<PyMappingProxy>()?;
let mut ret = collections::BTreeMap::new();
for (k, v) in dict {
for res in mappingproxy.try_iter()? {
let (k, v) = res?;
ret.insert(k.extract()?, v.extract()?);
}
Ok(ret)
Expand Down
1 change: 1 addition & 0 deletions src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions src/sealed.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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> {}
Expand Down
2 changes: 1 addition & 1 deletion src/types/dict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ where
}

/// Represents a tuple which can be used as a PyDict item.
trait PyDictItem<'py> {
pub trait PyDictItem<'py> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a possibly unintentional change

Suggested change
pub trait PyDictItem<'py> {
trait PyDictItem<'py> {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed! For some reason I thought I needed this for one of the tests.

type K: IntoPyObject<'py>;
type V: IntoPyObject<'py>;
fn unpack(self) -> (Self::K, Self::V);
Expand Down
Loading
Loading