Skip to content

Commit

Permalink
Merge pull request #644 from sebpuetz/fix-mapping-protocol
Browse files Browse the repository at this point in the history
Remove contains and iter from PyMappingProtocol.
  • Loading branch information
kngwyu authored Oct 26, 2019
2 parents 59975f8 + 6868d7f commit da1ab1b
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 83 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 4 additions & 25 deletions pyo3-derive-backend/src/defs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
58 changes: 0 additions & 58 deletions src/class/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -89,16 +75,6 @@ pub trait PyMappingDelItemProtocol<'p>: PyMappingProtocol<'p> {
type Result: Into<PyResult<()>>;
}

pub trait PyMappingIterProtocol<'p>: PyMappingProtocol<'p> {
type Success: IntoPy<PyObject>;
type Result: Into<PyResult<Self::Success>>;
}

pub trait PyMappingContainsProtocol<'p>: PyMappingProtocol<'p> {
type Value: FromPyObject<'p>;
type Result: Into<PyResult<bool>>;
}

pub trait PyMappingReversedProtocol<'p>: PyMappingProtocol<'p> {
type Success: IntoPy<PyObject>;
type Result: Into<PyResult<Self::Success>>;
Expand Down Expand Up @@ -142,12 +118,6 @@ where
fn methods() -> Vec<PyMethodDef> {
let mut methods = Vec::new();

if let Some(def) = <Self as PyMappingIterProtocolImpl>::__iter__() {
methods.push(def)
}
if let Some(def) = <Self as PyMappingContainsProtocolImpl>::__contains__() {
methods.push(def)
}
if let Some(def) = <Self as PyMappingReversedProtocolImpl>::__reversed__() {
methods.push(def)
}
Expand Down Expand Up @@ -283,20 +253,6 @@ where
}
}

#[doc(hidden)]
pub trait PyMappingContainsProtocolImpl {
fn __contains__() -> Option<PyMethodDef>;
}

impl<'p, T> PyMappingContainsProtocolImpl for T
where
T: PyMappingProtocol<'p>,
{
default fn __contains__() -> Option<PyMethodDef> {
None
}
}

#[doc(hidden)]
pub trait PyMappingReversedProtocolImpl {
fn __reversed__() -> Option<PyMethodDef>;
Expand All @@ -310,17 +266,3 @@ where
None
}
}

#[doc(hidden)]
pub trait PyMappingIterProtocolImpl {
fn __iter__() -> Option<PyMethodDef>;
}

impl<'p, T> PyMappingIterProtocolImpl for T
where
T: PyMappingProtocol<'p>,
{
default fn __iter__() -> Option<PyMethodDef> {
None
}
}
130 changes: 130 additions & 0 deletions tests/test_mapping.rs
Original file line number Diff line number Diff line change
@@ -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<String, usize>,
}

#[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<usize> {
Ok(self.index.len())
}

fn __getitem__(&self, query: String) -> PyResult<usize> {
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<PyObject> {
let gil = Python::acquire_gil();
Ok(self
.index
.keys()
.cloned()
.collect::<Vec<String>>()
.into_py(gil.python()))
}
}

#[test]
fn test_getitem() {
let gil = Python::acquire_gil();
let py = gil.python();
let d = [("Mapping", py.get_type::<Mapping>())].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::<Mapping>())].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::<Mapping>())].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::<Mapping>())].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'}");
}

0 comments on commit da1ab1b

Please sign in to comment.