diff --git a/CHANGELOG.md b/CHANGELOG.md index a3aff9b3..eece3190 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,8 +15,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * Fix handling of invalid utf-8 sequences in `PyString::as_bytes` [#639](https://github.com/PyO3/pyo3/pull/639) and `PyString::to_string_lossy` [#642](https://github.com/PyO3/pyo3/pull/642). +* Remove `__contains__` and `__iter__` from PyMappingProtocol. [#644](https://github.com/PyO3/pyo3/pull/644) * Fix proc-macro definition of PySetAttrProtocol. [#645](https://github.com/PyO3/pyo3/pull/645) + ## [0.8.1] ### Added diff --git a/pyo3-derive-backend/src/defs.rs b/pyo3-derive-backend/src/defs.rs index 04c1fa3b..e099f853 100644 --- a/pyo3-derive-backend/src/defs.rs +++ b/pyo3-derive-backend/src/defs.rs @@ -277,37 +277,16 @@ pub const MAPPING: Proto = Proto { pyres: false, proto: "pyo3::class::mapping::PyMappingDelItemProtocol", }, - MethodProto::Binary { - name: "__contains__", - arg: "Value", - pyres: false, - proto: "pyo3::class::mapping::PyMappingContainsProtocol", - }, MethodProto::Unary { name: "__reversed__", pyres: true, proto: "pyo3::class::mapping::PyMappingReversedProtocol", }, - MethodProto::Unary { - name: "__iter__", - pyres: true, - proto: "pyo3::class::mapping::PyMappingIterProtocol", - }, - ], - py_methods: &[ - PyMethod { - name: "__iter__", - proto: "pyo3::class::mapping::PyMappingIterProtocolImpl", - }, - PyMethod { - name: "__contains__", - proto: "pyo3::class::mapping::PyMappingContainsProtocolImpl", - }, - PyMethod { - name: "__reversed__", - proto: "pyo3::class::mapping::PyMappingReversedProtocolImpl", - }, ], + py_methods: &[PyMethod { + name: "__reversed__", + proto: "pyo3::class::mapping::PyMappingReversedProtocolImpl", + }], }; pub const SEQ: Proto = Proto { diff --git a/src/class/mapping.rs b/src/class/mapping.rs index 32326ff4..f1c4b3ff 100644 --- a/src/class/mapping.rs +++ b/src/class/mapping.rs @@ -43,20 +43,6 @@ pub trait PyMappingProtocol<'p>: PyTypeInfo { unimplemented!() } - fn __iter__(&'p self, py: Python<'p>) -> Self::Result - where - Self: PyMappingIterProtocol<'p>, - { - unimplemented!() - } - - fn __contains__(&'p self, value: Self::Value) -> Self::Result - where - Self: PyMappingContainsProtocol<'p>, - { - unimplemented!() - } - fn __reversed__(&'p self) -> Self::Result where Self: PyMappingReversedProtocol<'p>, @@ -89,16 +75,6 @@ pub trait PyMappingDelItemProtocol<'p>: PyMappingProtocol<'p> { type Result: Into>; } -pub trait PyMappingIterProtocol<'p>: PyMappingProtocol<'p> { - type Success: IntoPy; - type Result: Into>; -} - -pub trait PyMappingContainsProtocol<'p>: PyMappingProtocol<'p> { - type Value: FromPyObject<'p>; - type Result: Into>; -} - pub trait PyMappingReversedProtocol<'p>: PyMappingProtocol<'p> { type Success: IntoPy; type Result: Into>; @@ -142,12 +118,6 @@ where fn methods() -> Vec { let mut methods = Vec::new(); - if let Some(def) = ::__iter__() { - methods.push(def) - } - if let Some(def) = ::__contains__() { - methods.push(def) - } if let Some(def) = ::__reversed__() { methods.push(def) } @@ -283,20 +253,6 @@ where } } -#[doc(hidden)] -pub trait PyMappingContainsProtocolImpl { - fn __contains__() -> Option; -} - -impl<'p, T> PyMappingContainsProtocolImpl for T -where - T: PyMappingProtocol<'p>, -{ - default fn __contains__() -> Option { - None - } -} - #[doc(hidden)] pub trait PyMappingReversedProtocolImpl { fn __reversed__() -> Option; @@ -310,17 +266,3 @@ where None } } - -#[doc(hidden)] -pub trait PyMappingIterProtocolImpl { - fn __iter__() -> Option; -} - -impl<'p, T> PyMappingIterProtocolImpl for T -where - T: PyMappingProtocol<'p>, -{ - default fn __iter__() -> Option { - None - } -} diff --git a/tests/test_mapping.rs b/tests/test_mapping.rs new file mode 100644 index 00000000..6160da0f --- /dev/null +++ b/tests/test_mapping.rs @@ -0,0 +1,130 @@ +#![feature(specialization)] +use std::collections::HashMap; + +use pyo3::exceptions::KeyError; +use pyo3::prelude::*; +use pyo3::types::IntoPyDict; +use pyo3::types::PyList; +use pyo3::PyMappingProtocol; + +#[pyclass] +struct Mapping { + index: HashMap, +} + +#[pymethods] +impl Mapping { + #[new] + fn new(obj: &PyRawObject, elements: Option<&PyList>) -> PyResult<()> { + if let Some(pylist) = elements { + let mut elems = HashMap::with_capacity(pylist.len()); + for (i, pyelem) in pylist.into_iter().enumerate() { + let elem = String::extract(pyelem)?; + elems.insert(elem, i); + } + obj.init(Self { index: elems }); + } else { + obj.init(Self { + index: HashMap::new(), + }); + } + Ok(()) + } +} + +#[pyproto] +impl PyMappingProtocol for Mapping { + fn __len__(&self) -> PyResult { + Ok(self.index.len()) + } + + fn __getitem__(&self, query: String) -> PyResult { + self.index + .get(&query) + .copied() + .ok_or_else(|| KeyError::py_err("unknown key")) + } + + fn __setitem__(&mut self, key: String, value: usize) -> PyResult<()> { + self.index.insert(key, value); + Ok(()) + } + + fn __delitem__(&mut self, key: String) -> PyResult<()> { + if self.index.remove(&key).is_none() { + KeyError::py_err("unknown key").into() + } else { + Ok(()) + } + } + + /// not an actual reversed implementation, just to demonstrate that the method is callable. + fn __reversed__(&self) -> PyResult { + let gil = Python::acquire_gil(); + Ok(self + .index + .keys() + .cloned() + .collect::>() + .into_py(gil.python())) + } +} + +#[test] +fn test_getitem() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let d = [("Mapping", py.get_type::())].into_py_dict(py); + + let run = |code| py.run(code, None, Some(d)).unwrap(); + let err = |code| py.run(code, None, Some(d)).unwrap_err(); + + run("m = Mapping(['1', '2', '3']); assert m['1'] == 0"); + run("m = Mapping(['1', '2', '3']); assert m['2'] == 1"); + run("m = Mapping(['1', '2', '3']); assert m['3'] == 2"); + err("m = Mapping(['1', '2', '3']); print(m['4'])"); +} + +#[test] +fn test_setitem() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let d = [("Mapping", py.get_type::())].into_py_dict(py); + + let run = |code| py.run(code, None, Some(d)).unwrap(); + let err = |code| py.run(code, None, Some(d)).unwrap_err(); + + run("m = Mapping(['1', '2', '3']); m['1'] = 4; assert m['1'] == 4"); + run("m = Mapping(['1', '2', '3']); m['0'] = 0; assert m['0'] == 0"); + run("m = Mapping(['1', '2', '3']); len(m) == 4"); + err("m = Mapping(['1', '2', '3']); m[0] = 'hello'"); + err("m = Mapping(['1', '2', '3']); m[0] = -1"); +} + +#[test] +fn test_delitem() { + let gil = Python::acquire_gil(); + let py = gil.python(); + + let d = [("Mapping", py.get_type::())].into_py_dict(py); + let run = |code| py.run(code, None, Some(d)).unwrap(); + let err = |code| py.run(code, None, Some(d)).unwrap_err(); + + run( + "m = Mapping(['1', '2', '3']); del m['1']; assert len(m) == 2; \ + assert m['2'] == 1; assert m['3'] == 2", + ); + err("m = Mapping(['1', '2', '3']); del m[-1]"); + err("m = Mapping(['1', '2', '3']); del m['4']"); +} + +#[test] +fn test_reversed() { + let gil = Python::acquire_gil(); + let py = gil.python(); + + let d = [("Mapping", py.get_type::())].into_py_dict(py); + let run = |code| py.run(code, None, Some(d)).unwrap(); + + run("m = Mapping(['1', '2']); assert set(reversed(m)) == {'1', '2'}"); +}