From 51563bcf2f4097443ce7a9888c0168265137d894 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 18:26:05 +0100 Subject: [PATCH 01/18] Support frozenset, tuple as dict keys https://github.com/pybind/pybind11/discussions/3836 Add frozenset as a pybind11 type. Allow type_caster to be (explicitly) selected and specialized, defaulting to (inherit from) type_caster. Use type_caster for std::set, std::map etc. key converter. Specialize type_caster for std::set, std::vector etc. Add tests. --- include/pybind11/cast.h | 3 ++ include/pybind11/pytypes.h | 31 +++++++++++++---- include/pybind11/stl.h | 68 ++++++++++++++++++++++++++++---------- tests/test_stl.cpp | 14 ++++++++ tests/test_stl.py | 28 ++++++++++++++++ 5 files changed, 119 insertions(+), 25 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e8128710e2..1f77d7bed9 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -79,6 +79,9 @@ class type_caster> { explicit operator std::reference_wrapper() { return cast_op(subcaster); } }; +template +class type_caster : public type_caster {}; + #define PYBIND11_TYPE_CASTER(type, py_name) \ protected: \ type value; \ diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 324fa932f1..fdb4eecbc4 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1784,14 +1784,11 @@ class kwargs : public dict { PYBIND11_OBJECT_DEFAULT(kwargs, dict, PyDict_Check) }; -class set : public object { +class set_base : public object { +protected: + PYBIND11_OBJECT(set_base, object, PyAnySet_Check) + public: - PYBIND11_OBJECT_CVT(set, object, PySet_Check, PySet_New) - set() : object(PySet_New(nullptr), stolen_t{}) { - if (!m_ptr) { - pybind11_fail("Could not allocate set object!"); - } - } size_t size() const { return (size_t) PySet_Size(m_ptr); } bool empty() const { return size() == 0; } template @@ -1805,6 +1802,26 @@ class set : public object { } }; +class set : public set_base { +public: + PYBIND11_OBJECT_CVT(set, set_base, PySet_Check, PySet_New) + set() : set_base(PySet_New(nullptr), stolen_t{}) { + if (!m_ptr) { + pybind11_fail("Could not allocate set object!"); + } + } +}; + +class frozenset : public set_base { +public: + PYBIND11_OBJECT_CVT(frozenset, set_base, PyFrozenSet_Check, PyFrozenSet_New) + frozenset() : set_base(PyFrozenSet_New(nullptr), stolen_t{}) { + if (!m_ptr) { + pybind11_fail("Could not allocate frozenset object!"); + } + } +}; + class function : public object { public: PYBIND11_OBJECT_DEFAULT(function, object, PyCallable_Check) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 51b57a92ba..315bb98a53 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -49,23 +49,23 @@ forwarded_type forward_like(U &&u) { return std::forward>(std::forward(u)); } -template +template struct set_caster { using type = Type; - using key_conv = make_caster; + using key_conv = type_caster>; bool load(handle src, bool convert) { - if (!isinstance(src)) { + if (!isinstance(src)) { return false; } - auto s = reinterpret_borrow(src); + auto s = reinterpret_borrow(src); value.clear(); for (auto entry : s) { key_conv conv; if (!conv.load(entry, convert)) { return false; } - value.insert(cast_op(std::move(conv))); + value.insert(std::move(conv).operator typename key_conv::template cast_op_type()); } return true; } @@ -75,7 +75,7 @@ struct set_caster { if (!std::is_lvalue_reference::value) { policy = return_value_policy_override::policy(policy); } - pybind11::set s; + typename std::conditional::type s; for (auto &&value : src) { auto value_ = reinterpret_steal( key_conv::cast(forward_like(value), policy, parent)); @@ -86,12 +86,13 @@ struct set_caster { return s.release(); } - PYBIND11_TYPE_CASTER(type, const_name("Set[") + key_conv::name + const_name("]")); + PYBIND11_TYPE_CASTER(type, + const_name("FrozenSet[", "Set[") + key_conv::name + const_name("]")); }; template struct map_caster { - using key_conv = make_caster; + using key_conv = type_caster>; using value_conv = make_caster; bool load(handle src, bool convert) { @@ -106,7 +107,9 @@ struct map_caster { if (!kconv.load(it.first.ptr(), convert) || !vconv.load(it.second.ptr(), convert)) { return false; } - value.emplace(cast_op(std::move(kconv)), cast_op(std::move(vconv))); + value.emplace( + std::move(kconv).operator typename key_conv::template cast_op_type(), + cast_op(std::move(vconv))); } return true; } @@ -138,7 +141,7 @@ struct map_caster { + const_name("]")); }; -template +template struct list_caster { using value_conv = make_caster; @@ -174,7 +177,7 @@ struct list_caster { if (!std::is_lvalue_reference::value) { policy = return_value_policy_override::policy(policy); } - list l(src.size()); + std::conditional_t l(src.size()); ssize_t index = 0; for (auto &&value : src) { auto value_ = reinterpret_steal( @@ -182,24 +185,38 @@ struct list_caster { if (!value_) { return handle(); } - PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference + if (Const) + PyTuple_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference + else + PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference } return l.release(); } - PYBIND11_TYPE_CASTER(Type, const_name("List[") + value_conv::name + const_name("]")); + PYBIND11_TYPE_CASTER(Type, + const_name("Tuple[", "List[") + value_conv::name + + const_name(", ...]", "]")); }; template struct type_caster> : list_caster, Type> {}; +template +struct type_caster> : list_caster, Type, true> {}; + template struct type_caster> : list_caster, Type> {}; +template +struct type_caster> : list_caster, Type, true> {}; + template struct type_caster> : list_caster, Type> {}; -template +template +struct type_caster> : list_caster, Type, true> {}; + +template struct array_caster { using value_conv = make_caster; @@ -238,7 +255,7 @@ struct array_caster { template static handle cast(T &&src, return_value_policy policy, handle parent) { - list l(src.size()); + std::conditional_t l(src.size()); ssize_t index = 0; for (auto &&value : src) { auto value_ = reinterpret_steal( @@ -246,23 +263,30 @@ struct array_caster { if (!value_) { return handle(); } - PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference + if (Const) + PyTuple_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference + else + PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference } return l.release(); } PYBIND11_TYPE_CASTER(ArrayType, - const_name("List[") + value_conv::name + const_name("Tuple[", "List[") + value_conv::name + const_name(const_name(""), const_name("[") + const_name() + const_name("]")) - + const_name("]")); + + const_name(", ...]", "]")); }; template struct type_caster> : array_caster, Type, false, Size> {}; +template +struct type_caster> + : array_caster, Type, false, Size, true> {}; + template struct type_caster> : array_caster, Type, true> {}; @@ -270,10 +294,18 @@ template struct type_caster> : set_caster, Key> {}; +template +struct type_caster> + : set_caster, Key, true> {}; + template struct type_caster> : set_caster, Key> {}; +template +struct type_caster> + : set_caster, Key, true> {}; + template struct type_caster> : map_caster, Key, Value> {}; diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index b56a91953b..fb52dd41a0 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -248,6 +248,20 @@ TEST_SUBMODULE(stl, m) { return v; }); + // test_frozen_key + m.def("cast_set_map", []() { return std::map, std::string>{{{"key1", "key2"}, "value"}}; }); + m.def("load_set_map", [](const std::map, std::string> &map) { + return map.at({"key1", "key2"}) == "value" && map.at({"key3"}) == "value2"; + }); + m.def("cast_set_set", []() { return std::set>{{"key1", "key2"}}; }); + m.def("load_set_set", [](const std::set> &set) { + return (set.count({"key1", "key2"}) != 0u) && (set.count({"key3"}) != 0u); + }); + m.def("cast_vector_set", []() { return std::set>{{1, 2}}; }); + m.def("load_vector_set", [](const std::set> &set) { + return (set.count({1, 2}) != 0u) && (set.count({3}) != 0u); + }); + pybind11::enum_(m, "EnumType") .value("kSet", EnumType::kSet) .value("kUnset", EnumType::kUnset); diff --git a/tests/test_stl.py b/tests/test_stl.py index 3dc55230ab..cf09804770 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -94,6 +94,34 @@ def test_recursive_casting(): assert z[0].value == 7 and z[1].value == 42 +def test_frozen_key(doc): + """Test that we special-case C++ key types to Python immutable containers, e.g.: + std::map, V> <-> dict[frozenset[K], V] + std::set> <-> set[frozenset[T]] + std::set> <-> set[tuple[T, ...]] + """ + s = m.cast_set_map() + assert s == {frozenset({"key1", "key2"}): "value"} + s[frozenset({"key3"})] = "value2" + assert m.load_set_map(s) + assert doc(m.cast_set_map) == "cast_set_map() -> Dict[FrozenSet[str], str]" + assert doc(m.load_set_map) == "load_set_map(arg0: Dict[FrozenSet[str], str]) -> bool" + + s = m.cast_set_set() + assert s == {frozenset({"key1", "key2"})} + s.add(frozenset({"key3"})) + assert m.load_set_set(s) + assert doc(m.cast_set_set) == "cast_set_set() -> Set[FrozenSet[str]]" + assert doc(m.load_set_set) == "load_set_set(arg0: Set[FrozenSet[str]]) -> bool" + + s = m.cast_vector_set() + assert s == {(1, 2)} + s.add((3,)) + assert m.load_vector_set(s) + assert doc(m.cast_vector_set) == "cast_vector_set() -> Set[Tuple[int, ...]]" + assert doc(m.load_vector_set) == "load_vector_set(arg0: Set[Tuple[int, ...]]) -> bool" + + def test_move_out_container(): """Properties use the `reference_internal` policy by default. If the underlying function returns an rvalue, the policy is automatically changed to `move` to avoid referencing From d0f9f2e3e5e2b8493318608e493fe442ebebc350 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 19 Apr 2022 17:36:17 +0000 Subject: [PATCH 02/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- include/pybind11/stl.h | 15 ++++++++++----- tests/test_stl.cpp | 4 +++- tests/test_stl.py | 8 ++++++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 315bb98a53..d39f2aea1f 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -65,7 +65,8 @@ struct set_caster { if (!conv.load(entry, convert)) { return false; } - value.insert(std::move(conv).operator typename key_conv::template cast_op_type()); + value.insert( + std::move(conv).operator typename key_conv::template cast_op_type()); } return true; } @@ -87,7 +88,8 @@ struct set_caster { } PYBIND11_TYPE_CASTER(type, - const_name("FrozenSet[", "Set[") + key_conv::name + const_name("]")); + const_name("FrozenSet[", "Set[") + key_conv::name + + const_name("]")); }; template @@ -202,19 +204,22 @@ template struct type_caster> : list_caster, Type> {}; template -struct type_caster> : list_caster, Type, true> {}; +struct type_caster> + : list_caster, Type, true> {}; template struct type_caster> : list_caster, Type> {}; template -struct type_caster> : list_caster, Type, true> {}; +struct type_caster> + : list_caster, Type, true> {}; template struct type_caster> : list_caster, Type> {}; template -struct type_caster> : list_caster, Type, true> {}; +struct type_caster> + : list_caster, Type, true> {}; template struct array_caster { diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index fb52dd41a0..d818bcc6eb 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -249,7 +249,9 @@ TEST_SUBMODULE(stl, m) { }); // test_frozen_key - m.def("cast_set_map", []() { return std::map, std::string>{{{"key1", "key2"}, "value"}}; }); + m.def("cast_set_map", []() { + return std::map, std::string>{{{"key1", "key2"}, "value"}}; + }); m.def("load_set_map", [](const std::map, std::string> &map) { return map.at({"key1", "key2"}) == "value" && map.at({"key3"}) == "value2"; }); diff --git a/tests/test_stl.py b/tests/test_stl.py index cf09804770..3dc984a124 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -105,7 +105,9 @@ def test_frozen_key(doc): s[frozenset({"key3"})] = "value2" assert m.load_set_map(s) assert doc(m.cast_set_map) == "cast_set_map() -> Dict[FrozenSet[str], str]" - assert doc(m.load_set_map) == "load_set_map(arg0: Dict[FrozenSet[str], str]) -> bool" + assert ( + doc(m.load_set_map) == "load_set_map(arg0: Dict[FrozenSet[str], str]) -> bool" + ) s = m.cast_set_set() assert s == {frozenset({"key1", "key2"})} @@ -119,7 +121,9 @@ def test_frozen_key(doc): s.add((3,)) assert m.load_vector_set(s) assert doc(m.cast_vector_set) == "cast_vector_set() -> Set[Tuple[int, ...]]" - assert doc(m.load_vector_set) == "load_vector_set(arg0: Set[Tuple[int, ...]]) -> bool" + assert ( + doc(m.load_vector_set) == "load_vector_set(arg0: Set[Tuple[int, ...]]) -> bool" + ) def test_move_out_container(): From 038904a5329e96e430fe1390a5e41bc06e61994b Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 18:39:52 +0100 Subject: [PATCH 03/18] Fix for C++11 mode --- include/pybind11/stl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 315bb98a53..304dc4056e 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -177,7 +177,7 @@ struct list_caster { if (!std::is_lvalue_reference::value) { policy = return_value_policy_override::policy(policy); } - std::conditional_t l(src.size()); + conditional_t l(src.size()); ssize_t index = 0; for (auto &&value : src) { auto value_ = reinterpret_steal( @@ -255,7 +255,7 @@ struct array_caster { template static handle cast(T &&src, return_value_policy policy, handle parent) { - std::conditional_t l(src.size()); + conditional_t l(src.size()); ssize_t index = 0; for (auto &&value : src) { auto value_ = reinterpret_steal( From a56f91c2ce4f36349b417c097c2f113f9719f338 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 21:39:25 +0100 Subject: [PATCH 04/18] protect non-const methods s.t. they aren't public on frozenset --- include/pybind11/pytypes.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fdb4eecbc4..23d5b56742 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1787,15 +1787,15 @@ class kwargs : public dict { class set_base : public object { protected: PYBIND11_OBJECT(set_base, object, PyAnySet_Check) - -public: - size_t size() const { return (size_t) PySet_Size(m_ptr); } - bool empty() const { return size() == 0; } template bool add(T &&val) /* py-non-const */ { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } void clear() /* py-non-const */ { PySet_Clear(m_ptr); } + +public: + size_t size() const { return (size_t) PySet_Size(m_ptr); } + bool empty() const { return size() == 0; } template bool contains(T &&val) const { return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; From cd09b3aa9848705423094da032feaa704518d3c3 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 21:43:01 +0100 Subject: [PATCH 05/18] formatting --- include/pybind11/stl.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index fa95153977..10c85da869 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -187,10 +187,11 @@ struct list_caster { if (!value_) { return handle(); } - if (Const) + if (Const) { PyTuple_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference - else + } else { PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference + } } return l.release(); } @@ -268,10 +269,11 @@ struct array_caster { if (!value_) { return handle(); } - if (Const) + if (Const) { PyTuple_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference - else + } else { PyList_SET_ITEM(l.ptr(), index++, value_.release().ptr()); // steals a reference + } } return l.release(); } From 6c045a5ae272c7313ac6e8e602770e7069173930 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 22:44:35 +0100 Subject: [PATCH 06/18] Revert "protect non-const methods" This reverts commit a56f91c2ce4f36349b417c097c2f113f9719f338. --- include/pybind11/pytypes.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 23d5b56742..fdb4eecbc4 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1787,15 +1787,15 @@ class kwargs : public dict { class set_base : public object { protected: PYBIND11_OBJECT(set_base, object, PyAnySet_Check) + +public: + size_t size() const { return (size_t) PySet_Size(m_ptr); } + bool empty() const { return size() == 0; } template bool add(T &&val) /* py-non-const */ { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } void clear() /* py-non-const */ { PySet_Clear(m_ptr); } - -public: - size_t size() const { return (size_t) PySet_Size(m_ptr); } - bool empty() const { return size() == 0; } template bool contains(T &&val) const { return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; From bb123d1d2b080bdfad7fc01fe5168c25cfea3e85 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 22:46:12 +0100 Subject: [PATCH 07/18] Revert "Revert "protect non-const methods"" This reverts commit 6c045a5ae272c7313ac6e8e602770e7069173930. --- include/pybind11/pytypes.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fdb4eecbc4..23d5b56742 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1787,15 +1787,15 @@ class kwargs : public dict { class set_base : public object { protected: PYBIND11_OBJECT(set_base, object, PyAnySet_Check) - -public: - size_t size() const { return (size_t) PySet_Size(m_ptr); } - bool empty() const { return size() == 0; } template bool add(T &&val) /* py-non-const */ { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } void clear() /* py-non-const */ { PySet_Clear(m_ptr); } + +public: + size_t size() const { return (size_t) PySet_Size(m_ptr); } + bool empty() const { return size() == 0; } template bool contains(T &&val) const { return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; From 6b55983207d9e0ad557cf35264f723b46f42bb20 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 22:59:03 +0100 Subject: [PATCH 08/18] Move add() and clear() to set call PySet_Add directly in caster --- include/pybind11/pytypes.h | 10 +++++----- include/pybind11/stl.h | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 23d5b56742..fa2475d6ab 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1787,11 +1787,6 @@ class kwargs : public dict { class set_base : public object { protected: PYBIND11_OBJECT(set_base, object, PyAnySet_Check) - template - bool add(T &&val) /* py-non-const */ { - return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; - } - void clear() /* py-non-const */ { PySet_Clear(m_ptr); } public: size_t size() const { return (size_t) PySet_Size(m_ptr); } @@ -1810,6 +1805,11 @@ class set : public set_base { pybind11_fail("Could not allocate set object!"); } } + template + bool add(T &&val) /* py-non-const */ { + return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; + } + void clear() /* py-non-const */ { PySet_Clear(m_ptr); } }; class frozenset : public set_base { diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 10c85da869..b6abbaa2be 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -80,7 +80,8 @@ struct set_caster { for (auto &&value : src) { auto value_ = reinterpret_steal( key_conv::cast(forward_like(value), policy, parent)); - if (!value_ || !s.add(std::move(value_))) { + // pybind11::frozenset doesn't have add() for safety, so call PySet_Add directly. + if (!value_ || PySet_Add(s.ptr(), detail::object_or_cast(std::move(value_)).ptr()) != 0) { return handle(); } } From 3eb1a8b45d08ed9b0e4decb780b0bc808a3c77d7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 19 Apr 2022 21:59:57 +0000 Subject: [PATCH 09/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- include/pybind11/stl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index b6abbaa2be..bb4fadd227 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -81,7 +81,8 @@ struct set_caster { auto value_ = reinterpret_steal( key_conv::cast(forward_like(value), policy, parent)); // pybind11::frozenset doesn't have add() for safety, so call PySet_Add directly. - if (!value_ || PySet_Add(s.ptr(), detail::object_or_cast(std::move(value_)).ptr()) != 0) { + if (!value_ + || PySet_Add(s.ptr(), detail::object_or_cast(std::move(value_)).ptr()) != 0) { return handle(); } } From 1b7a941fecf94f3e3a583d5b23ab6c9a379a49fb Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Thu, 21 Apr 2022 02:08:11 +0100 Subject: [PATCH 10/18] Only use const type_caster for class types --- include/pybind11/stl.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index bb4fadd227..de169d0e75 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -49,10 +49,14 @@ forwarded_type forward_like(U &&u) { return std::forward>(std::forward(u)); } +template +using make_key_caster = type_caster>::value, + const intrinsic_t, intrinsic_t>>; + template struct set_caster { using type = Type; - using key_conv = type_caster>; + using key_conv = make_key_caster; bool load(handle src, bool convert) { if (!isinstance(src)) { @@ -96,7 +100,7 @@ struct set_caster { template struct map_caster { - using key_conv = type_caster>; + using key_conv = make_key_caster; using value_conv = make_caster; bool load(handle src, bool convert) { From 0ebef3bab36bd3cce1a6c0974fcc657261616e0c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 21 Apr 2022 01:08:54 +0000 Subject: [PATCH 11/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- include/pybind11/stl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index de169d0e75..b396aba0a3 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -51,7 +51,8 @@ forwarded_type forward_like(U &&u) { template using make_key_caster = type_caster>::value, - const intrinsic_t, intrinsic_t>>; + const intrinsic_t, + intrinsic_t>>; template struct set_caster { From fcaff44f09b936fb933d55c839863413ce55cc8c Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Thu, 21 Apr 2022 02:09:22 +0100 Subject: [PATCH 12/18] More tests for tuple -> list, frozenset -> set --- tests/test_stl.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_stl.py b/tests/test_stl.py index 3dc984a124..11a445019b 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -14,6 +14,7 @@ def test_vector(doc): assert m.cast_bool_vector() == [True, False] assert m.load_bool_vector([True, False]) + assert m.load_bool_vector(tuple([True, False])) assert doc(m.cast_vector) == "cast_vector() -> List[int]" assert doc(m.load_vector) == "load_vector(arg0: List[int]) -> bool" @@ -36,6 +37,7 @@ def test_array(doc): lst = m.cast_array() assert lst == [1, 2] assert m.load_array(lst) + assert m.load_array(tuple(lst)) assert doc(m.cast_array) == "cast_array() -> List[int[2]]" assert doc(m.load_array) == "load_array(arg0: List[int[2]]) -> bool" @@ -46,6 +48,7 @@ def test_valarray(doc): lst = m.cast_valarray() assert lst == [1, 4, 9] assert m.load_valarray(lst) + assert m.load_valarray(tuple(lst)) assert doc(m.cast_valarray) == "cast_valarray() -> List[int]" assert doc(m.load_valarray) == "load_valarray(arg0: List[int]) -> bool" @@ -70,6 +73,7 @@ def test_set(doc): assert s == {"key1", "key2"} s.add("key3") assert m.load_set(s) + assert m.load_set(frozenset(s)) assert doc(m.cast_set) == "cast_set() -> Set[str]" assert doc(m.load_set) == "load_set(arg0: Set[str]) -> bool" From e8280318664b9ff27a8bc306a13ef2d5ebc200a2 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Sun, 24 Apr 2022 21:54:59 +0100 Subject: [PATCH 13/18] Add frozenset, and allow it cast to std::set For the reverse direction, std::set still casts to set. This is in concordance with the behavior for sequence containers, where e.g. tuple casts to std::vector but std::vector casts to list. Extracted from #3886. --- include/pybind11/pytypes.h | 33 +++++++++++++++++++++++++-------- include/pybind11/stl.h | 4 ++-- tests/test_stl.py | 1 + 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 324fa932f1..fa2475d6ab 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1784,24 +1784,41 @@ class kwargs : public dict { PYBIND11_OBJECT_DEFAULT(kwargs, dict, PyDict_Check) }; -class set : public object { +class set_base : public object { +protected: + PYBIND11_OBJECT(set_base, object, PyAnySet_Check) + +public: + size_t size() const { return (size_t) PySet_Size(m_ptr); } + bool empty() const { return size() == 0; } + template + bool contains(T &&val) const { + return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; + } +}; + +class set : public set_base { public: - PYBIND11_OBJECT_CVT(set, object, PySet_Check, PySet_New) - set() : object(PySet_New(nullptr), stolen_t{}) { + PYBIND11_OBJECT_CVT(set, set_base, PySet_Check, PySet_New) + set() : set_base(PySet_New(nullptr), stolen_t{}) { if (!m_ptr) { pybind11_fail("Could not allocate set object!"); } } - size_t size() const { return (size_t) PySet_Size(m_ptr); } - bool empty() const { return size() == 0; } template bool add(T &&val) /* py-non-const */ { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } void clear() /* py-non-const */ { PySet_Clear(m_ptr); } - template - bool contains(T &&val) const { - return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; +}; + +class frozenset : public set_base { +public: + PYBIND11_OBJECT_CVT(frozenset, set_base, PyFrozenSet_Check, PyFrozenSet_New) + frozenset() : set_base(PyFrozenSet_New(nullptr), stolen_t{}) { + if (!m_ptr) { + pybind11_fail("Could not allocate frozenset object!"); + } } }; diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 51b57a92ba..838c2dc341 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -55,10 +55,10 @@ struct set_caster { using key_conv = make_caster; bool load(handle src, bool convert) { - if (!isinstance(src)) { + if (!isinstance(src)) { return false; } - auto s = reinterpret_borrow(src); + auto s = reinterpret_borrow(src); value.clear(); for (auto entry : s) { key_conv conv; diff --git a/tests/test_stl.py b/tests/test_stl.py index 3dc55230ab..2c858e3f5c 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -70,6 +70,7 @@ def test_set(doc): assert s == {"key1", "key2"} s.add("key3") assert m.load_set(s) + assert m.load_set(frozenset(s)) assert doc(m.cast_set) == "cast_set() -> Set[str]" assert doc(m.load_set) == "load_set(arg0: Set[str]) -> bool" From f2db7bb14e6ef601c4085b791027c7ca9a5c67ea Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Sun, 24 Apr 2022 21:56:33 +0100 Subject: [PATCH 14/18] Rename set_base to any_set to match Python C API since this will be part of pybind11 public API --- include/pybind11/pytypes.h | 16 ++++++++-------- include/pybind11/stl.h | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fa2475d6ab..0322235c92 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1784,9 +1784,9 @@ class kwargs : public dict { PYBIND11_OBJECT_DEFAULT(kwargs, dict, PyDict_Check) }; -class set_base : public object { +class any_set : public object { protected: - PYBIND11_OBJECT(set_base, object, PyAnySet_Check) + PYBIND11_OBJECT(any_set, object, PyAnySet_Check) public: size_t size() const { return (size_t) PySet_Size(m_ptr); } @@ -1797,10 +1797,10 @@ class set_base : public object { } }; -class set : public set_base { +class set : public any_set { public: - PYBIND11_OBJECT_CVT(set, set_base, PySet_Check, PySet_New) - set() : set_base(PySet_New(nullptr), stolen_t{}) { + PYBIND11_OBJECT_CVT(set, any_set, PySet_Check, PySet_New) + set() : any_set(PySet_New(nullptr), stolen_t{}) { if (!m_ptr) { pybind11_fail("Could not allocate set object!"); } @@ -1812,10 +1812,10 @@ class set : public set_base { void clear() /* py-non-const */ { PySet_Clear(m_ptr); } }; -class frozenset : public set_base { +class frozenset : public any_set { public: - PYBIND11_OBJECT_CVT(frozenset, set_base, PyFrozenSet_Check, PyFrozenSet_New) - frozenset() : set_base(PyFrozenSet_New(nullptr), stolen_t{}) { + PYBIND11_OBJECT_CVT(frozenset, any_set, PyFrozenSet_Check, PyFrozenSet_New) + frozenset() : any_set(PyFrozenSet_New(nullptr), stolen_t{}) { if (!m_ptr) { pybind11_fail("Could not allocate frozenset object!"); } diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 838c2dc341..0d67176bdf 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -55,10 +55,10 @@ struct set_caster { using key_conv = make_caster; bool load(handle src, bool convert) { - if (!isinstance(src)) { + if (!isinstance(src)) { return false; } - auto s = reinterpret_borrow(src); + auto s = reinterpret_borrow(src); value.clear(); for (auto entry : s) { key_conv conv; From ef92aa5e17aff1e587163779f89825adaf4c8fa5 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Sun, 1 May 2022 12:17:40 +0100 Subject: [PATCH 15/18] PR: static_cast, anyset --- include/pybind11/pytypes.h | 18 +++++++++--------- include/pybind11/stl.h | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 0322235c92..963cc564e2 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1784,12 +1784,12 @@ class kwargs : public dict { PYBIND11_OBJECT_DEFAULT(kwargs, dict, PyDict_Check) }; -class any_set : public object { +class anyset : public object { protected: - PYBIND11_OBJECT(any_set, object, PyAnySet_Check) + PYBIND11_OBJECT(anyset, object, PyAnySet_Check) public: - size_t size() const { return (size_t) PySet_Size(m_ptr); } + size_t size() const { return static_cast(PySet_Size(m_ptr)); } bool empty() const { return size() == 0; } template bool contains(T &&val) const { @@ -1797,10 +1797,10 @@ class any_set : public object { } }; -class set : public any_set { +class set : public anyset { public: - PYBIND11_OBJECT_CVT(set, any_set, PySet_Check, PySet_New) - set() : any_set(PySet_New(nullptr), stolen_t{}) { + PYBIND11_OBJECT_CVT(set, anyset, PySet_Check, PySet_New) + set() : anyset(PySet_New(nullptr), stolen_t{}) { if (!m_ptr) { pybind11_fail("Could not allocate set object!"); } @@ -1812,10 +1812,10 @@ class set : public any_set { void clear() /* py-non-const */ { PySet_Clear(m_ptr); } }; -class frozenset : public any_set { +class frozenset : public anyset { public: - PYBIND11_OBJECT_CVT(frozenset, any_set, PyFrozenSet_Check, PyFrozenSet_New) - frozenset() : any_set(PyFrozenSet_New(nullptr), stolen_t{}) { + PYBIND11_OBJECT_CVT(frozenset, anyset, PyFrozenSet_Check, PyFrozenSet_New) + frozenset() : anyset(PyFrozenSet_New(nullptr), stolen_t{}) { if (!m_ptr) { pybind11_fail("Could not allocate frozenset object!"); } diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 0d67176bdf..625fb210fc 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -55,10 +55,10 @@ struct set_caster { using key_conv = make_caster; bool load(handle src, bool convert) { - if (!isinstance(src)) { + if (!isinstance(src)) { return false; } - auto s = reinterpret_borrow(src); + auto s = reinterpret_borrow(src); value.clear(); for (auto entry : s) { key_conv conv; From 736f293de6e966cca784a8277db9cc08fc74677f Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Sun, 1 May 2022 12:34:58 +0100 Subject: [PATCH 16/18] Add tests for frozenset and rename anyset methods --- tests/test_pytypes.cpp | 19 ++++++++++++----- tests/test_pytypes.py | 48 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index d1e9b81a73..9347385789 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -75,7 +75,7 @@ TEST_SUBMODULE(pytypes, m) { m.def("get_none", [] { return py::none(); }); m.def("print_none", [](const py::none &none) { py::print("none: {}"_s.format(none)); }); - // test_set + // test_set, test_frozenset m.def("get_set", []() { py::set set; set.add(py::str("key1")); @@ -83,14 +83,23 @@ TEST_SUBMODULE(pytypes, m) { set.add(std::string("key3")); return set; }); - m.def("print_set", [](const py::set &set) { + m.def("get_frozenset", []() { + py::set set; + set.add(py::str("key1")); + set.add("key2"); + set.add(std::string("key3")); + return py::frozenset(set); + }); + m.def("print_anyset", [](const py::anyset &set) { for (auto item : set) { py::print("key:", item); } }); - m.def("set_contains", - [](const py::set &set, const py::object &key) { return set.contains(key); }); - m.def("set_contains", [](const py::set &set, const char *key) { return set.contains(key); }); + m.def("anyset_size", [](const py::anyset &set) { return set.size(); }); + m.def("anyset_empty", [](const py::anyset &set) { return set.empty(); }); + m.def("anyset_contains", + [](const py::anyset &set, const py::object &key) { return set.contains(key); }); + m.def("anyset_contains", [](const py::anyset &set, const char *key) { return set.contains(key); }); // test_dict m.def("get_dict", []() { return py::dict("key"_a = "value"); }); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 5c715ada6b..65233f7f97 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -66,11 +66,12 @@ def test_none(capture, doc): def test_set(capture, doc): s = m.get_set() + assert isinstance(s, set) assert s == {"key1", "key2", "key3"} with capture: s.add("key4") - m.print_set(s) + m.print_anyset(s) assert ( capture.unordered == """ @@ -81,12 +82,47 @@ def test_set(capture, doc): """ ) - assert not m.set_contains(set(), 42) - assert m.set_contains({42}, 42) - assert m.set_contains({"foo"}, "foo") + assert m.anyset_size(set() == 0 + assert m.anyset_size(s) == 4 - assert doc(m.get_list) == "get_list() -> list" - assert doc(m.print_list) == "print_list(arg0: list) -> None" + assert m.anyset_empty(set()) + assert not m.anyset_empty({42}) + + assert not m.anyset_contains(set(), 42) + assert m.anyset_contains({42}, 42) + assert m.anyset_contains({"foo"}, "foo") + + assert doc(m.get_set) == "get_set() -> set" + assert doc(m.print_anyset) == "print_anyset(arg0: set) -> None" + + +def test_frozenset(capture, doc): + s = m.get_frozenset() + assert isinstance(s, frozenset) + assert s == frozenset({"key1", "key2", "key3"}) + + with capture: + m.print_anyset(s) + assert ( + capture.unordered + == """ + key: key1 + key: key2 + key: key3 + """ + ) + + assert m.anyset_size(frozenset()) == 0 + assert m.anyset_size(s) == 3 + + assert m.anyset_empty(frozenset()) + assert not m.anyset_empty(frozenset({42})) + + assert not m.anyset_contains(frozenset(), 42) + assert m.anyset_contains(frozenset({42}), 42) + assert m.anyset_contains(frozenset({"foo"}), "foo") + + assert doc(m.get_frozenset) == "get_frozenset() -> frozenset" def test_dict(capture, doc): From faf8a519a2e6b92c59dbee62271c0c75effbc29b Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Sun, 1 May 2022 16:01:06 +0100 Subject: [PATCH 17/18] Remove frozenset default ctor, add tests Making frozenset non-default constructible means that we need to adjust pyobject_caster to not require that its value is default constructible, by initializing value to a nil handle. This also allows writing C++ functions taking anyset, and is arguably a performance improvement, since there is no need to allocate an object that will just be replaced by load. Add some more tests, including anyset::empty, anyset::size, set::add and set::clear. --- include/pybind11/cast.h | 6 ++++++ include/pybind11/pytypes.h | 5 ----- tests/test_pytypes.cpp | 4 ++++ tests/test_pytypes.py | 19 ++++++++----------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e8128710e2..7d1e943dee 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -907,6 +907,12 @@ struct handle_type_name { template struct pyobject_caster { + template ::value, int> = 0> + pyobject_caster() : value() {} + + template ::value, int> = 0> + pyobject_caster() : value(reinterpret_steal(handle())) {} + template ::value, int> = 0> bool load(handle src, bool /* convert */) { value = src; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 963cc564e2..a79b0caadf 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1815,11 +1815,6 @@ class set : public anyset { class frozenset : public anyset { public: PYBIND11_OBJECT_CVT(frozenset, anyset, PyFrozenSet_Check, PyFrozenSet_New) - frozenset() : anyset(PyFrozenSet_New(nullptr), stolen_t{}) { - if (!m_ptr) { - pybind11_fail("Could not allocate frozenset object!"); - } - } }; class function : public object { diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 9347385789..1f26625d14 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -100,6 +100,8 @@ TEST_SUBMODULE(pytypes, m) { m.def("anyset_contains", [](const py::anyset &set, const py::object &key) { return set.contains(key); }); m.def("anyset_contains", [](const py::anyset &set, const char *key) { return set.contains(key); }); + m.def("set_add", [](py::set &set, const py::object &key) { set.add(key); }); + m.def("set_clear", [](py::set &set) { set.clear(); }); // test_dict m.def("get_dict", []() { return py::dict("key"_a = "value"); }); @@ -319,6 +321,7 @@ TEST_SUBMODULE(pytypes, m) { "list"_a = py::list(d["list"]), "dict"_a = py::dict(d["dict"]), "set"_a = py::set(d["set"]), + "frozenset"_a = py::frozenset(d["frozenset"]), "memoryview"_a = py::memoryview(d["memoryview"])); }); @@ -334,6 +337,7 @@ TEST_SUBMODULE(pytypes, m) { "list"_a = d["list"].cast(), "dict"_a = d["dict"].cast(), "set"_a = d["set"].cast(), + "frozenset"_a = d["frozenset"].cast(), "memoryview"_a = d["memoryview"].cast()); }); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 65233f7f97..a6adfdddad 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -69,8 +69,8 @@ def test_set(capture, doc): assert isinstance(s, set) assert s == {"key1", "key2", "key3"} + s.add("key4") with capture: - s.add("key4") m.print_anyset(s) assert ( capture.unordered @@ -82,18 +82,18 @@ def test_set(capture, doc): """ ) - assert m.anyset_size(set() == 0 - assert m.anyset_size(s) == 4 + m.set_add(s, "key5") + assert m.anyset_size(s) == 5 - assert m.anyset_empty(set()) - assert not m.anyset_empty({42}) + m.set_clear(s) + assert m.anyset_empty(s) assert not m.anyset_contains(set(), 42) assert m.anyset_contains({42}, 42) assert m.anyset_contains({"foo"}, "foo") assert doc(m.get_set) == "get_set() -> set" - assert doc(m.print_anyset) == "print_anyset(arg0: set) -> None" + assert doc(m.print_anyset) == "print_anyset(arg0: anyset) -> None" def test_frozenset(capture, doc): @@ -111,12 +111,8 @@ def test_frozenset(capture, doc): key: key3 """ ) - - assert m.anyset_size(frozenset()) == 0 assert m.anyset_size(s) == 3 - - assert m.anyset_empty(frozenset()) - assert not m.anyset_empty(frozenset({42})) + assert not m.anyset_empty(s) assert not m.anyset_contains(frozenset(), 42) assert m.anyset_contains(frozenset({42}), 42) @@ -338,6 +334,7 @@ def test_constructors(): list: range(3), dict: [("two", 2), ("one", 1), ("three", 3)], set: [4, 4, 5, 6, 6, 6], + frozenset: [4, 4, 5, 6, 6, 6], memoryview: b"abc", } inputs = {k.__name__: v for k, v in data.items()} From 05b6147ffb3d548ff79d70f985ef3231af5ef617 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 1 May 2022 15:01:45 +0000 Subject: [PATCH 18/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_pytypes.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 1f26625d14..8d296f655a 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -99,7 +99,8 @@ TEST_SUBMODULE(pytypes, m) { m.def("anyset_empty", [](const py::anyset &set) { return set.empty(); }); m.def("anyset_contains", [](const py::anyset &set, const py::object &key) { return set.contains(key); }); - m.def("anyset_contains", [](const py::anyset &set, const char *key) { return set.contains(key); }); + m.def("anyset_contains", + [](const py::anyset &set, const char *key) { return set.contains(key); }); m.def("set_add", [](py::set &set, const py::object &key) { set.add(key); }); m.def("set_clear", [](py::set &set) { set.clear(); });