From a4b56286cee9d401c24e74fff457359c5142ddb6 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Tue, 19 Apr 2022 18:26:05 +0100 Subject: [PATCH 01/15] Support frozenset, tuple as dict keys https://github.com/pybind/pybind11/discussions/3836 Add frozenset as a pybind11 type. Add freeze() function converting set to frozenset and list to tuple; use it in std::set and std::map casters. Add tests. --- include/pybind11/cast.h | 19 +++++++++++++++++++ include/pybind11/pytypes.h | 33 +++++++++++++++++++++++++-------- include/pybind11/stl.h | 21 ++++++++++++++------- tests/test_stl.cpp | 16 ++++++++++++++++ tests/test_stl.py | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 15 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e8128710e2..15db0296ea 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1617,6 +1617,25 @@ object object_api::call(Args &&...args) const { return operator()(std::forward(args)...); } +// Convert list -> tuple and set -> frozenset for use as keys in dict, set etc. +// https://mail.python.org/pipermail/python-dev/2005-October/057586.html +inline object freeze(object &&obj) { + if (isinstance(obj)) { + return tuple(std::move(obj)); + } else if (isinstance(obj)) { + return frozenset(std::move(obj)); + } else { + return std::move(obj); + } +} + +template +constexpr inline auto get_frozen_name_impl(int) { return Caster::frozen_name; } +template +constexpr inline auto get_frozen_name_impl(long) { return Caster::name; } +template +constexpr inline auto get_frozen_name() { return get_frozen_name_impl(0); } + PYBIND11_NAMESPACE_END(detail) template 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..7c15b60b8a 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; @@ -79,14 +79,15 @@ 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_))) { + if (!value_ || !s.add(freeze(std::move(value_)))) { return handle(); } } return s.release(); } - PYBIND11_TYPE_CASTER(type, const_name("Set[") + key_conv::name + const_name("]")); + PYBIND11_TYPE_CASTER(type, const_name("Set[") + get_frozen_name() + const_name("]")); + static constexpr auto frozen_name = const_name("FrozenSet[") + get_frozen_name() + const_name("]"); }; template @@ -128,14 +129,14 @@ struct map_caster { if (!key || !value) { return handle(); } - d[key] = value; + d[freeze(std::move(key))] = std::move(value); } return d.release(); } PYBIND11_TYPE_CASTER(Type, - const_name("Dict[") + key_conv::name + const_name(", ") + value_conv::name - + const_name("]")); + const_name("Dict[") + get_frozen_name() + const_name(", ") + + value_conv::name + const_name("]")); }; template @@ -188,6 +189,7 @@ struct list_caster { } PYBIND11_TYPE_CASTER(Type, const_name("List[") + value_conv::name + const_name("]")); + static constexpr auto frozen_name = const_name("Tuple[") + value_conv::name + const_name(", ...]"); }; template @@ -257,6 +259,11 @@ struct array_caster { const_name("[") + const_name() + const_name("]")) + const_name("]")); + static constexpr auto frozen_name = const_name("Tuple[") + value_conv::name + + const_name(const_name(", ..."), + const_name("[") + const_name() + + const_name("]")) + + const_name("]"); }; template diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index b56a91953b..d818bcc6eb 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -248,6 +248,22 @@ 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..3dc984a124 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -94,6 +94,38 @@ 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 1a0fa088a7bc0350cddf36f4a74f00972f5c7886 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 20 Apr 2022 00:32:32 +0000 Subject: [PATCH 02/15] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- include/pybind11/cast.h | 12 +++++++++--- include/pybind11/stl.h | 16 +++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 15db0296ea..583e0f1d11 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1630,11 +1630,17 @@ inline object freeze(object &&obj) { } template -constexpr inline auto get_frozen_name_impl(int) { return Caster::frozen_name; } +constexpr inline auto get_frozen_name_impl(int) { + return Caster::frozen_name; +} template -constexpr inline auto get_frozen_name_impl(long) { return Caster::name; } +constexpr inline auto get_frozen_name_impl(long) { + return Caster::name; +} template -constexpr inline auto get_frozen_name() { return get_frozen_name_impl(0); } +constexpr inline auto get_frozen_name() { + return get_frozen_name_impl(0); +} PYBIND11_NAMESPACE_END(detail) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 7c15b60b8a..972d56e088 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -87,7 +87,8 @@ struct set_caster { } PYBIND11_TYPE_CASTER(type, const_name("Set[") + get_frozen_name() + const_name("]")); - static constexpr auto frozen_name = const_name("FrozenSet[") + get_frozen_name() + const_name("]"); + static constexpr auto frozen_name + = const_name("FrozenSet[") + get_frozen_name() + const_name("]"); }; template @@ -189,7 +190,8 @@ struct list_caster { } PYBIND11_TYPE_CASTER(Type, const_name("List[") + value_conv::name + const_name("]")); - static constexpr auto frozen_name = const_name("Tuple[") + value_conv::name + const_name(", ...]"); + static constexpr auto frozen_name + = const_name("Tuple[") + value_conv::name + const_name(", ...]"); }; template @@ -259,11 +261,11 @@ struct array_caster { const_name("[") + const_name() + const_name("]")) + const_name("]")); - static constexpr auto frozen_name = const_name("Tuple[") + value_conv::name - + const_name(const_name(", ..."), - const_name("[") + const_name() - + const_name("]")) - + const_name("]"); + static constexpr auto frozen_name + = const_name("Tuple[") + value_conv::name + + const_name(const_name(", ..."), + const_name("[") + const_name() + const_name("]")) + + const_name("]"); }; template From ce1dff96ca1bd4d6027ccdd8a72cc3fc5fd656f0 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Wed, 20 Apr 2022 01:36:05 +0100 Subject: [PATCH 03/15] C++11 --- include/pybind11/cast.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 583e0f1d11..2a829a1695 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1629,12 +1629,12 @@ inline object freeze(object &&obj) { } } -template -constexpr inline auto get_frozen_name_impl(int) { +template +constexpr inline decltype(Caster::frozen_name) get_frozen_name_impl(int) { return Caster::frozen_name; } template -constexpr inline auto get_frozen_name_impl(long) { +constexpr inline decltype(Caster::name) get_frozen_name_impl(long) { return Caster::name; } template From 26a2df5d27619014ce9774df07170152abd512fd Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Wed, 20 Apr 2022 01:41:53 +0100 Subject: [PATCH 04/15] C++11, again --- include/pybind11/cast.h | 16 +++++----------- include/pybind11/stl.h | 6 +++--- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 2a829a1695..906773ac1d 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1629,18 +1629,12 @@ inline object freeze(object &&obj) { } } +template +struct frozen_type_name { static constexpr auto name = Caster::name; }; template -constexpr inline decltype(Caster::frozen_name) get_frozen_name_impl(int) { - return Caster::frozen_name; -} -template -constexpr inline decltype(Caster::name) get_frozen_name_impl(long) { - return Caster::name; -} -template -constexpr inline auto get_frozen_name() { - return get_frozen_name_impl(0); -} +struct frozen_type_name> { + static constexpr auto name = Caster::frozen_name; +}; PYBIND11_NAMESPACE_END(detail) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 972d56e088..8f60f195dc 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -86,9 +86,9 @@ struct set_caster { return s.release(); } - PYBIND11_TYPE_CASTER(type, const_name("Set[") + get_frozen_name() + const_name("]")); + PYBIND11_TYPE_CASTER(type, const_name("Set[") + frozen_type_name::name + const_name("]")); static constexpr auto frozen_name - = const_name("FrozenSet[") + get_frozen_name() + const_name("]"); + = const_name("FrozenSet[") + frozen_type_name::name + const_name("]"); }; template @@ -136,7 +136,7 @@ struct map_caster { } PYBIND11_TYPE_CASTER(Type, - const_name("Dict[") + get_frozen_name() + const_name(", ") + const_name("Dict[") + frozen_type_name::name + const_name(", ") + value_conv::name + const_name("]")); }; From a7cabffdd7bf76cacec0d984520df5e8b0b8f046 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 20 Apr 2022 00:42:30 +0000 Subject: [PATCH 05/15] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- include/pybind11/cast.h | 4 +++- include/pybind11/stl.h | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 906773ac1d..e38583b15d 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1630,7 +1630,9 @@ inline object freeze(object &&obj) { } template -struct frozen_type_name { static constexpr auto name = Caster::name; }; +struct frozen_type_name { + static constexpr auto name = Caster::name; +}; template struct frozen_type_name> { static constexpr auto name = Caster::frozen_name; diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 8f60f195dc..0652becabe 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -86,7 +86,8 @@ struct set_caster { return s.release(); } - PYBIND11_TYPE_CASTER(type, const_name("Set[") + frozen_type_name::name + const_name("]")); + PYBIND11_TYPE_CASTER(type, + const_name("Set[") + frozen_type_name::name + const_name("]")); static constexpr auto frozen_name = const_name("FrozenSet[") + frozen_type_name::name + const_name("]"); }; From c82d7a1721cbbc8513a9c1a8b9c5469827113635 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Wed, 20 Apr 2022 01:51:58 +0100 Subject: [PATCH 06/15] clang-tidy --- include/pybind11/cast.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 906773ac1d..5336d33c7d 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1622,11 +1622,11 @@ object object_api::call(Args &&...args) const { inline object freeze(object &&obj) { if (isinstance(obj)) { return tuple(std::move(obj)); - } else if (isinstance(obj)) { + } + if (isinstance(obj)) { return frozenset(std::move(obj)); - } else { - return std::move(obj); } + return std::move(obj); } template From 43ca8f5a0c29693aa5bfd6158557c379ad43f4e8 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Thu, 21 Apr 2022 02:09:22 +0100 Subject: [PATCH 07/15] 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 08/15] 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 09/15] 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 10/15] 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 11/15] 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 12/15] 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 13/15] [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(); }); From 0cf2caef08fb016a7a512a1249875724805496c9 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Mon, 2 May 2022 20:22:04 +0100 Subject: [PATCH 14/15] Add rationale to `pyobject_caster` default ctor --- include/pybind11/cast.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 7d1e943dee..80fb23fe54 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -910,6 +910,8 @@ struct pyobject_caster { template ::value, int> = 0> pyobject_caster() : value() {} + // `type` may not be default constructible (e.g. frozenset, anyset). Initializing `value` + // to a nil handle is safe since it will only be accessed if `load` succeeds. template ::value, int> = 0> pyobject_caster() : value(reinterpret_steal(handle())) {} From c63ca46a5e20a035fe9f810549f8310e685e1be0 Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Wed, 4 May 2022 22:32:50 +0100 Subject: [PATCH 15/15] Remove ineffectual protected: access control --- include/pybind11/pytypes.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a79b0caadf..256a2441b1 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1785,10 +1785,8 @@ class kwargs : public dict { }; class anyset : public object { -protected: - PYBIND11_OBJECT(anyset, object, PyAnySet_Check) - public: + PYBIND11_OBJECT(anyset, object, PyAnySet_Check) size_t size() const { return static_cast(PySet_Size(m_ptr)); } bool empty() const { return size() == 0; } template