From 872f9ff346e65281fb7084a0f463e03a1bd8e3ac Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Sun, 16 Mar 2025 22:01:21 +0100 Subject: [PATCH 01/13] Updated STL type hints use support collections.abc --- include/pybind11/stl.h | 16 +++++++++++----- tests/test_kwargs_and_defaults.py | 2 +- tests/test_pytypes.py | 2 +- tests/test_stl.py | 22 ++++++++++++++-------- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 6a148e7402..2a56814ca9 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -203,7 +203,9 @@ struct set_caster { return s.release(); } - PYBIND11_TYPE_CASTER(type, const_name("set[") + key_conv::name + const_name("]")); + PYBIND11_TYPE_CASTER(type, + io_name("collections.abc.Set", "set") + const_name("[") + key_conv::name + + const_name("]")); }; template @@ -274,7 +276,8 @@ struct map_caster { } PYBIND11_TYPE_CASTER(Type, - const_name("dict[") + key_conv::name + const_name(", ") + value_conv::name + io_name("collections.abc.Mapping", "dict") + const_name("[") + + key_conv::name + const_name(", ") + value_conv::name + const_name("]")); }; @@ -340,7 +343,9 @@ struct list_caster { return l.release(); } - PYBIND11_TYPE_CASTER(Type, const_name("list[") + value_conv::name + const_name("]")); + PYBIND11_TYPE_CASTER(Type, + io_name("collections.abc.Sequence", "list") + const_name("[") + + value_conv::name + const_name("]")); }; template @@ -474,8 +479,9 @@ struct array_caster { using cast_op_type = movable_cast_op_type; static constexpr auto name - = const_name(const_name(""), const_name("Annotated[")) + const_name("list[") - + value_conv::name + const_name("]") + = const_name(const_name(""), const_name("Annotated[")) + + io_name("collections.abc.Sequence", "list") + const_name("[") + value_conv::name + + const_name("]") + const_name( const_name(""), const_name(", FixedSize(") + const_name() + const_name(")]")); }; diff --git a/tests/test_kwargs_and_defaults.py b/tests/test_kwargs_and_defaults.py index a8e19f15bb..b62e4b7412 100644 --- a/tests/test_kwargs_and_defaults.py +++ b/tests/test_kwargs_and_defaults.py @@ -22,7 +22,7 @@ def test_function_signatures(doc): assert doc(m.kw_func3) == "kw_func3(data: str = 'Hello world!') -> None" assert ( doc(m.kw_func4) - == "kw_func4(myList: list[typing.SupportsInt] = [13, 17]) -> str" + == "kw_func4(myList: collections.abc.Sequence[typing.SupportsInt] = [13, 17]) -> str" ) assert ( doc(m.kw_func_udl) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index deb4b06d41..1e857ad580 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -1250,7 +1250,7 @@ def test_arg_return_type_hints(doc): # std::vector assert ( doc(m.half_of_number_vector) - == "half_of_number_vector(arg0: list[Union[float, int]]) -> list[float]" + == "half_of_number_vector(arg0: collections.abc.Sequence[Union[float, int]]) -> list[float]" ) # Tuple assert ( diff --git a/tests/test_stl.py b/tests/test_stl.py index 29a6bf119f..cd19b4ec3c 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -20,7 +20,10 @@ def test_vector(doc): assert m.load_bool_vector((True, False)) assert doc(m.cast_vector) == "cast_vector() -> list[int]" - assert doc(m.load_vector) == "load_vector(arg0: list[typing.SupportsInt]) -> bool" + assert ( + doc(m.load_vector) + == "load_vector(arg0: collections.abc.Sequence[typing.SupportsInt]) -> bool" + ) # Test regression caused by 936: pointers to stl containers weren't castable assert m.cast_ptr_vector() == ["lvalue", "lvalue"] @@ -45,7 +48,7 @@ def test_array(doc): assert doc(m.cast_array) == "cast_array() -> Annotated[list[int], FixedSize(2)]" assert ( doc(m.load_array) - == "load_array(arg0: Annotated[list[typing.SupportsInt], FixedSize(2)]) -> bool" + == "load_array(arg0: Annotated[collections.abc.Sequence[typing.SupportsInt], FixedSize(2)]) -> bool" ) @@ -65,7 +68,8 @@ def test_valarray(doc): assert doc(m.cast_valarray) == "cast_valarray() -> list[int]" assert ( - doc(m.load_valarray) == "load_valarray(arg0: list[typing.SupportsInt]) -> bool" + doc(m.load_valarray) + == "load_valarray(arg0: collections.abc.Sequence[typing.SupportsInt]) -> bool" ) @@ -79,7 +83,9 @@ def test_map(doc): assert m.load_map(d) assert doc(m.cast_map) == "cast_map() -> dict[str, str]" - assert doc(m.load_map) == "load_map(arg0: dict[str, str]) -> bool" + assert ( + doc(m.load_map) == "load_map(arg0: collections.abc.Mapping[str, str]) -> bool" + ) def test_set(doc): @@ -91,7 +97,7 @@ def test_set(doc): 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" + assert doc(m.load_set) == "load_set(arg0: collections.abc.Set[str]) -> bool" def test_recursive_casting(): @@ -273,7 +279,7 @@ def __fspath__(self): assert m.parent_paths(["foo/bar", "foo/baz"]) == [Path("foo"), Path("foo")] assert ( doc(m.parent_paths) - == "parent_paths(arg0: list[Union[os.PathLike, str, bytes]]) -> list[pathlib.Path]" + == "parent_paths(arg0: collections.abc.Sequence[Union[os.PathLike, str, bytes]]) -> list[pathlib.Path]" ) # py::typing::List assert m.parent_paths_list(["foo/bar", "foo/baz"]) == [Path("foo"), Path("foo")] @@ -364,7 +370,7 @@ def test_stl_pass_by_pointer(msg): msg(excinfo.value) == """ stl_pass_by_pointer(): incompatible function arguments. The following argument types are supported: - 1. (v: list[typing.SupportsInt] = None) -> list[int] + 1. (v: collections.abc.Sequence[typing.SupportsInt] = None) -> list[int] Invoked with: """ @@ -376,7 +382,7 @@ def test_stl_pass_by_pointer(msg): msg(excinfo.value) == """ stl_pass_by_pointer(): incompatible function arguments. The following argument types are supported: - 1. (v: list[typing.SupportsInt] = None) -> list[int] + 1. (v: collections.abc.Sequence[typing.SupportsInt] = None) -> list[int] Invoked with: None """ From 80a9086405d3657dc9ba20063c43d02faf1f5ba8 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Sun, 16 Mar 2025 23:04:32 +0100 Subject: [PATCH 02/13] Updated array_caster to match numpy/eigen typing.Annotated stlye --- include/pybind11/stl.h | 7 ++++--- tests/test_stl.py | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 2a56814ca9..d2182b9dff 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -479,11 +479,12 @@ struct array_caster { using cast_op_type = movable_cast_op_type; static constexpr auto name - = const_name(const_name(""), const_name("Annotated[")) + = const_name(const_name(""), const_name("typing.Annotated[")) + io_name("collections.abc.Sequence", "list") + const_name("[") + value_conv::name + const_name("]") - + const_name( - const_name(""), const_name(", FixedSize(") + const_name() + const_name(")]")); + + const_name(const_name(""), + const_name(", \"FixedSize(") + const_name() + + const_name(")\"]")); }; template diff --git a/tests/test_stl.py b/tests/test_stl.py index cd19b4ec3c..7bbdaf20d9 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -45,10 +45,13 @@ def test_array(doc): assert m.load_array(lst) assert m.load_array(tuple(lst)) - assert doc(m.cast_array) == "cast_array() -> Annotated[list[int], FixedSize(2)]" + assert ( + doc(m.cast_array) + == 'cast_array() -> typing.Annotated[list[int], "FixedSize(2)"]' + ) assert ( doc(m.load_array) - == "load_array(arg0: Annotated[collections.abc.Sequence[typing.SupportsInt], FixedSize(2)]) -> bool" + == 'load_array(arg0: typing.Annotated[collections.abc.Sequence[typing.SupportsInt], "FixedSize(2)"]) -> bool' ) From caa2277d07234e1cdbe444303f8630e7c55558d2 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Fri, 21 Mar 2025 01:21:10 +0100 Subject: [PATCH 03/13] Added support for Mapping, Set and Sequence derived from collections.abc. --- include/pybind11/stl.h | 18 +++++- tests/test_stl.cpp | 3 + tests/test_stl.py | 126 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 2 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index d2182b9dff..e698b8dfcf 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -173,7 +173,14 @@ struct set_caster { public: bool load(handle src, bool convert) { if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { - return false; + if (!convert) { + return false; + } + if (!(isinstance(src, module_::import("collections.abc").attr("Set")) + && hasattr(src, "__contains__") && hasattr(src, "__iter__") + && hasattr(src, "__len__"))) { + return false; + } } if (isinstance(src)) { value.clear(); @@ -237,7 +244,14 @@ struct map_caster { public: bool load(handle src, bool convert) { if (!PyObjectTypeIsConvertibleToStdMap(src.ptr())) { - return false; + if (!convert) { + return false; + } + if (!(isinstance(src, module_::import("collections.abc").attr("Mapping")) + && hasattr(src, "__getitem__") && hasattr(src, "__iter__") + && hasattr(src, "__len__"))) { + return false; + } } if (isinstance(src)) { return convert_elements(reinterpret_borrow(src), convert); diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 9ddd951e0c..41ff72a9fd 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -648,4 +648,7 @@ TEST_SUBMODULE(stl, m) { } return zum; }); + m.def("roundtrip_std_vector_int", [](const std::vector &v) { return v; }); + m.def("roundtrip_std_map_str_int", [](const std::map &m) { return m; }); + m.def("roundtrip_std_set_int", [](const std::set &s) { return s; }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 7bbdaf20d9..ffac67e2a8 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -576,3 +576,129 @@ def gen_invalid(): with pytest.raises(expected_exception): m.pass_std_map_int(FakePyMappingGenObj(gen_obj)) assert not tuple(gen_obj) + + +def test_sequence_caster_protocol(doc): + from collections.abc import Sequence + + class SequenceLike(Sequence): + def __init__(self, *args): + self.data = tuple(args) + + def __len__(self): + return len(self.data) + + def __getitem__(self, index): + return self.data[index] + + class FakeSequenceLike: + def __init__(self, *args): + self.data = tuple(args) + + def __len__(self): + return len(self.data) + + def __getitem__(self, index): + return self.data[index] + + assert ( + doc(m.roundtrip_std_vector_int) + == "roundtrip_std_vector_int(arg0: collections.abc.Sequence[int]) -> list[int]" + ) + assert m.roundtrip_std_vector_int([1, 2, 3]) == [1, 2, 3] + assert m.roundtrip_std_vector_int((1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int(SequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int(FakeSequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int([]) == [] + assert m.roundtrip_std_vector_int(()) == [] + assert m.roundtrip_std_vector_int(FakeSequenceLike()) == [] + + +def test_mapping_caster_protocol(doc): + from collections.abc import Mapping + + class MappingLike(Mapping): + def __init__(self, **kwargs): + self.data = dict(kwargs) + + def __len__(self): + return len(self.data) + + def __getitem__(self, key): + return self.data[key] + + def __iter__(self): + yield from self.data + + class FakeMappingLike: + def __init__(self, **kwargs): + self.data = dict(kwargs) + + def __len__(self): + return len(self.data) + + def __getitem__(self, key): + return self.data[key] + + def __iter__(self): + yield from self.data + + assert ( + doc(m.roundtrip_std_map_str_int) + == "roundtrip_std_map_str_int(arg0: collections.abc.Mapping[str, int]) -> dict[str, int]" + ) + assert m.roundtrip_std_map_str_int({"a": 1, "b": 2, "c": 3}) == { + "a": 1, + "b": 2, + "c": 3, + } + assert m.roundtrip_std_map_str_int(MappingLike(a=1, b=2, c=3)) == { + "a": 1, + "b": 2, + "c": 3, + } + assert m.roundtrip_std_map_str_int({}) == {} + assert m.roundtrip_std_map_str_int(MappingLike()) == {} + with pytest.raises(TypeError): + m.roundtrip_std_map_str_int(FakeMappingLike(a=1, b=2, c=3)) + + +def test_set_caster_protocol(doc): + from collections.abc import Set + + class SetLike(Set): + def __init__(self, *args): + self.data = set(args) + + def __len__(self): + return len(self.data) + + def __contains__(self, item): + return item in self.data + + def __iter__(self): + yield from self.data + + class FakeSetLike: + def __init__(self, *args): + self.data = set(args) + + def __len__(self): + return len(self.data) + + def __contains__(self, item): + return item in self.data + + def __iter__(self): + yield from self.data + + assert ( + doc(m.roundtrip_std_set_int) + == "roundtrip_std_set_int(arg0: collections.abc.Set[int]) -> set[int]" + ) + assert m.roundtrip_std_set_int({1, 2, 3}) == {1, 2, 3} + assert m.roundtrip_std_set_int(SetLike(1, 2, 3)) == {1, 2, 3} + assert m.roundtrip_std_set_int(set()) == set() + assert m.roundtrip_std_set_int(SetLike()) == set() + with pytest.raises(TypeError): + m.roundtrip_std_set_int(FakeSetLike(1, 2, 3)) From 71c78461ab1c581238ea38659e7855b5b80abc66 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Fri, 21 Mar 2025 01:37:05 +0100 Subject: [PATCH 04/13] Fixed merge of typing.SupportsInt in new tests --- tests/test_stl.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_stl.py b/tests/test_stl.py index ffac67e2a8..cac33c15db 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -603,7 +603,7 @@ def __getitem__(self, index): assert ( doc(m.roundtrip_std_vector_int) - == "roundtrip_std_vector_int(arg0: collections.abc.Sequence[int]) -> list[int]" + == "roundtrip_std_vector_int(arg0: collections.abc.Sequence[typing.SupportsInt]) -> list[int]" ) assert m.roundtrip_std_vector_int([1, 2, 3]) == [1, 2, 3] assert m.roundtrip_std_vector_int((1, 2, 3)) == [1, 2, 3] @@ -645,7 +645,7 @@ def __iter__(self): assert ( doc(m.roundtrip_std_map_str_int) - == "roundtrip_std_map_str_int(arg0: collections.abc.Mapping[str, int]) -> dict[str, int]" + == "roundtrip_std_map_str_int(arg0: collections.abc.Mapping[str, typing.SupportsInt]) -> dict[str, int]" ) assert m.roundtrip_std_map_str_int({"a": 1, "b": 2, "c": 3}) == { "a": 1, @@ -694,7 +694,7 @@ def __iter__(self): assert ( doc(m.roundtrip_std_set_int) - == "roundtrip_std_set_int(arg0: collections.abc.Set[int]) -> set[int]" + == "roundtrip_std_set_int(arg0: collections.abc.Set[typing.SupportsInt]) -> set[int]" ) assert m.roundtrip_std_set_int({1, 2, 3}) == {1, 2, 3} assert m.roundtrip_std_set_int(SetLike(1, 2, 3)) == {1, 2, 3} From f5e48f178d94e28d6693cde35d2e23bd0b202748 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Mon, 24 Mar 2025 23:20:15 +0100 Subject: [PATCH 05/13] Integrated collections.abc checks into convertible check functions. --- include/pybind11/stl.h | 71 +++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index e698b8dfcf..57aad41e04 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -85,37 +85,44 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, return false; } -inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { - if (PySequence_Check(obj) != 0) { - return !PyUnicode_Check(obj) && !PyBytes_Check(obj); +inline bool HandleIsConvertibleToStdVector(const handle &src) { + if (PySequence_Check(src.ptr()) != 0) { + return !PyUnicode_Check(src.ptr()) && !PyBytes_Check(src.ptr()); } - return (PyGen_Check(obj) != 0) || (PyAnySet_Check(obj) != 0) + return (PyGen_Check(src.ptr()) != 0) || (PyAnySet_Check(src.ptr()) != 0) || PyObjectIsInstanceWithOneOfTpNames( - obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); + src.ptr(), {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } -inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { - return (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); +inline bool HandleIsConvertibleToStdSet(const handle &src, bool convert) { + return ((PyAnySet_Check(src.ptr()) != 0) + || PyObjectIsInstanceWithOneOfTpNames(src.ptr(), {"dict_keys"})) + || (convert && isinstance(src, module_::import("collections.abc").attr("Set")) + && hasattr(src, "__contains__") && hasattr(src, "__iter__") + && hasattr(src, "__len__")); } -inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { - if (PyDict_Check(obj)) { +inline bool HandleIsConvertibleToStdMap(const handle &src, bool convert) { + if (PyDict_Check(src.ptr())) { return true; } // Implicit requirement in the conditions below: // A type with `.__getitem__()` & `.items()` methods must implement these // to be compatible with https://docs.python.org/3/c-api/mapping.html - if (PyMapping_Check(obj) == 0) { - return false; - } - PyObject *items = PyObject_GetAttrString(obj, "items"); - if (items == nullptr) { - PyErr_Clear(); - return false; + if (PyMapping_Check(src.ptr()) != 0) { + PyObject *items = PyObject_GetAttrString(src.ptr(), "items"); + if (items != nullptr) { + bool is_convertible = (PyCallable_Check(items) != 0); + Py_DECREF(items); + if (is_convertible) { + return true; + } + } else { + PyErr_Clear(); + } } - bool is_convertible = (PyCallable_Check(items) != 0); - Py_DECREF(items); - return is_convertible; + return (convert && isinstance(src, module_::import("collections.abc").attr("Mapping")) + && hasattr(src, "__getitem__") && hasattr(src, "__iter__") && hasattr(src, "__len__")); } // @@ -172,15 +179,8 @@ struct set_caster { public: bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { - if (!convert) { - return false; - } - if (!(isinstance(src, module_::import("collections.abc").attr("Set")) - && hasattr(src, "__contains__") && hasattr(src, "__iter__") - && hasattr(src, "__len__"))) { - return false; - } + if (!HandleIsConvertibleToStdSet(src, convert)) { + return false; } if (isinstance(src)) { value.clear(); @@ -243,15 +243,8 @@ struct map_caster { public: bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdMap(src.ptr())) { - if (!convert) { - return false; - } - if (!(isinstance(src, module_::import("collections.abc").attr("Mapping")) - && hasattr(src, "__getitem__") && hasattr(src, "__iter__") - && hasattr(src, "__len__"))) { - return false; - } + if (!HandleIsConvertibleToStdMap(src, convert)) { + return false; } if (isinstance(src)) { return convert_elements(reinterpret_borrow(src), convert); @@ -300,7 +293,7 @@ struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { + if (!HandleIsConvertibleToStdVector(src)) { return false; } if (isinstance(src)) { @@ -435,7 +428,7 @@ struct array_caster { public: bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { + if (!HandleIsConvertibleToStdVector(src)) { return false; } if (isinstance(src)) { From 3d1a335fe78144b3718b99ffc5568f8713ed52e0 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Fri, 4 Apr 2025 23:06:49 +0200 Subject: [PATCH 06/13] Changed type hint of py::buffer to collections.abc.Buffer --- include/pybind11/cast.h | 2 +- tests/test_buffers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 89d3d43a7a..0750b36f84 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1229,7 +1229,7 @@ struct handle_type_name { }; template <> struct handle_type_name { - static constexpr auto name = const_name("Buffer"); + static constexpr auto name = const_name("collections.abc.Buffer"); }; template <> struct handle_type_name { diff --git a/tests/test_buffers.py b/tests/test_buffers.py index 2612edb270..d335b71e96 100644 --- a/tests/test_buffers.py +++ b/tests/test_buffers.py @@ -230,7 +230,7 @@ def test_ctypes_from_buffer(): def test_buffer_docstring(): assert ( m.get_buffer_info.__doc__.strip() - == "get_buffer_info(arg0: Buffer) -> pybind11_tests.buffers.buffer_info" + == "get_buffer_info(arg0: collections.abc.Buffer) -> pybind11_tests.buffers.buffer_info" ) From d6e1fb094c0216999abd50f822ee2695d7685990 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Fri, 4 Apr 2025 23:16:36 +0200 Subject: [PATCH 07/13] Changed convertible check function names --- include/pybind11/stl.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 57aad41e04..64bc0b684c 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -43,7 +43,7 @@ PYBIND11_NAMESPACE_BEGIN(detail) // Begin: Equivalent of // https://github.com/google/clif/blob/ae4eee1de07cdf115c0c9bf9fec9ff28efce6f6c/clif/python/runtime.cc#L388-L438 /* -The three `PyObjectTypeIsConvertibleTo*()` functions below are +The three `object_is_convertible_to_*()` functions below are the result of converging the behaviors of pybind11 and PyCLIF (http://github.com/google/clif). @@ -71,8 +71,8 @@ to prevent accidents and improve readability: such an enforcement would be more annoying than helpful. */ -inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, - std::initializer_list tp_names) { +inline bool object_is_instance_with_one_of_tp_names(PyObject *obj, + std::initializer_list tp_names) { if (PyType_Check(obj)) { return false; } @@ -85,24 +85,24 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, return false; } -inline bool HandleIsConvertibleToStdVector(const handle &src) { +inline bool object_is_convertible_to_std_vector(const handle &src) { if (PySequence_Check(src.ptr()) != 0) { return !PyUnicode_Check(src.ptr()) && !PyBytes_Check(src.ptr()); } return (PyGen_Check(src.ptr()) != 0) || (PyAnySet_Check(src.ptr()) != 0) - || PyObjectIsInstanceWithOneOfTpNames( + || object_is_instance_with_one_of_tp_names( src.ptr(), {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } -inline bool HandleIsConvertibleToStdSet(const handle &src, bool convert) { +inline bool object_is_convertible_to_std_set(const handle &src, bool convert) { return ((PyAnySet_Check(src.ptr()) != 0) - || PyObjectIsInstanceWithOneOfTpNames(src.ptr(), {"dict_keys"})) + || object_is_instance_with_one_of_tp_names(src.ptr(), {"dict_keys"})) || (convert && isinstance(src, module_::import("collections.abc").attr("Set")) && hasattr(src, "__contains__") && hasattr(src, "__iter__") && hasattr(src, "__len__")); } -inline bool HandleIsConvertibleToStdMap(const handle &src, bool convert) { +inline bool object_is_convertible_to_std_map(const handle &src, bool convert) { if (PyDict_Check(src.ptr())) { return true; } @@ -179,7 +179,7 @@ struct set_caster { public: bool load(handle src, bool convert) { - if (!HandleIsConvertibleToStdSet(src, convert)) { + if (!object_is_convertible_to_std_set(src, convert)) { return false; } if (isinstance(src)) { @@ -243,7 +243,7 @@ struct map_caster { public: bool load(handle src, bool convert) { - if (!HandleIsConvertibleToStdMap(src, convert)) { + if (!object_is_convertible_to_std_map(src, convert)) { return false; } if (isinstance(src)) { @@ -293,7 +293,7 @@ struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - if (!HandleIsConvertibleToStdVector(src)) { + if (!object_is_convertible_to_std_vector(src)) { return false; } if (isinstance(src)) { @@ -428,7 +428,7 @@ struct array_caster { public: bool load(handle src, bool convert) { - if (!HandleIsConvertibleToStdVector(src)) { + if (!object_is_convertible_to_std_vector(src)) { return false; } if (isinstance(src)) { From 68768f61d35ca20d501f256b1d3bfa8f1dc0e98e Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Fri, 4 Apr 2025 23:50:01 +0200 Subject: [PATCH 08/13] Added comments to convertible check functions --- include/pybind11/stl.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 64bc0b684c..2313744348 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -69,6 +69,9 @@ to prevent accidents and improve readability: are also fairly commonly used, therefore enforcing explicit conversions would have an unfavorable cost : benefit ratio; more sloppily speaking, such an enforcement would be more annoying than helpful. + +Additional checks have been added to allow types derived from `collections.abc.Set` and +`collections.abc.Mapping` (`collections.abc.Sequence` is already allowed by `PySequence_Check`). */ inline bool object_is_instance_with_one_of_tp_names(PyObject *obj, @@ -86,15 +89,19 @@ inline bool object_is_instance_with_one_of_tp_names(PyObject *obj, } inline bool object_is_convertible_to_std_vector(const handle &src) { + // Allow sequence-like objects, but not (byte-)string-like objects. if (PySequence_Check(src.ptr()) != 0) { return !PyUnicode_Check(src.ptr()) && !PyBytes_Check(src.ptr()); } + // Allow generators, set/frozenset and several common iterable types. return (PyGen_Check(src.ptr()) != 0) || (PyAnySet_Check(src.ptr()) != 0) || object_is_instance_with_one_of_tp_names( src.ptr(), {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } inline bool object_is_convertible_to_std_set(const handle &src, bool convert) { + // Allow set/frozenset, dict keys. + // In convert mode: also allow types derived from collections.abc.Set. return ((PyAnySet_Check(src.ptr()) != 0) || object_is_instance_with_one_of_tp_names(src.ptr(), {"dict_keys"})) || (convert && isinstance(src, module_::import("collections.abc").attr("Set")) @@ -103,12 +110,14 @@ inline bool object_is_convertible_to_std_set(const handle &src, bool convert) { } inline bool object_is_convertible_to_std_map(const handle &src, bool convert) { + // Allow dict. if (PyDict_Check(src.ptr())) { return true; } - // Implicit requirement in the conditions below: - // A type with `.__getitem__()` & `.items()` methods must implement these - // to be compatible with https://docs.python.org/3/c-api/mapping.html + // Allow types conforming to Mapping Protocol. + // According to https://docs.python.org/3/c-api/mapping.html, PyMappingCheck() checks for + // `__getitem__()` without checking the type of keys. In order to restrict the allowed types to + // actual Mapping-like types, we also check for `items()` method. if (PyMapping_Check(src.ptr()) != 0) { PyObject *items = PyObject_GetAttrString(src.ptr(), "items"); if (items != nullptr) { @@ -121,6 +130,7 @@ inline bool object_is_convertible_to_std_map(const handle &src, bool convert) { PyErr_Clear(); } } + // In convert mode: Allow types derived from collections.abc.Mapping return (convert && isinstance(src, module_::import("collections.abc").attr("Mapping")) && hasattr(src, "__getitem__") && hasattr(src, "__iter__") && hasattr(src, "__len__")); } From 5f771dadaad27f9fbf06f3bb4286f0d208120a54 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Sat, 5 Apr 2025 00:25:12 +0200 Subject: [PATCH 09/13] Removed checks for methods that are already required by the abstract base class --- include/pybind11/stl.h | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 2313744348..4258f1584c 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -100,13 +100,11 @@ inline bool object_is_convertible_to_std_vector(const handle &src) { } inline bool object_is_convertible_to_std_set(const handle &src, bool convert) { - // Allow set/frozenset, dict keys. + // Allow set/frozenset and dict keys. // In convert mode: also allow types derived from collections.abc.Set. return ((PyAnySet_Check(src.ptr()) != 0) || object_is_instance_with_one_of_tp_names(src.ptr(), {"dict_keys"})) - || (convert && isinstance(src, module_::import("collections.abc").attr("Set")) - && hasattr(src, "__contains__") && hasattr(src, "__iter__") - && hasattr(src, "__len__")); + || (convert && isinstance(src, module_::import("collections.abc").attr("Set"))); } inline bool object_is_convertible_to_std_map(const handle &src, bool convert) { @@ -115,9 +113,9 @@ inline bool object_is_convertible_to_std_map(const handle &src, bool convert) { return true; } // Allow types conforming to Mapping Protocol. - // According to https://docs.python.org/3/c-api/mapping.html, PyMappingCheck() checks for - // `__getitem__()` without checking the type of keys. In order to restrict the allowed types to - // actual Mapping-like types, we also check for `items()` method. + // According to https://docs.python.org/3/c-api/mapping.html, `PyMappingCheck()` checks for + // `__getitem__()` without checking the type of keys. In order to restrict the allowed types + // closer to actual Mapping-like types, we also check for the `items()` method. if (PyMapping_Check(src.ptr()) != 0) { PyObject *items = PyObject_GetAttrString(src.ptr(), "items"); if (items != nullptr) { @@ -131,8 +129,7 @@ inline bool object_is_convertible_to_std_map(const handle &src, bool convert) { } } // In convert mode: Allow types derived from collections.abc.Mapping - return (convert && isinstance(src, module_::import("collections.abc").attr("Mapping")) - && hasattr(src, "__getitem__") && hasattr(src, "__iter__") && hasattr(src, "__len__")); + return convert && isinstance(src, module_::import("collections.abc").attr("Mapping")); } // From 6fc2ab3b98cce7f6dbd30d58c761f4b21bc6f9e6 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Sun, 13 Apr 2025 23:00:33 +0200 Subject: [PATCH 10/13] Improved mapping caster test using more compact a1b2c3 variable --- tests/test_stl.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/test_stl.py b/tests/test_stl.py index cac33c15db..59d748e118 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -647,20 +647,13 @@ def __iter__(self): doc(m.roundtrip_std_map_str_int) == "roundtrip_std_map_str_int(arg0: collections.abc.Mapping[str, typing.SupportsInt]) -> dict[str, int]" ) - assert m.roundtrip_std_map_str_int({"a": 1, "b": 2, "c": 3}) == { - "a": 1, - "b": 2, - "c": 3, - } - assert m.roundtrip_std_map_str_int(MappingLike(a=1, b=2, c=3)) == { - "a": 1, - "b": 2, - "c": 3, - } + a1b2c3 = {"a": 1, "b": 2, "c": 3} + assert m.roundtrip_std_map_str_int(a1b2c3) == a1b2c3 + assert m.roundtrip_std_map_str_int(MappingLike(**a1b2c3)) == a1b2c3 assert m.roundtrip_std_map_str_int({}) == {} assert m.roundtrip_std_map_str_int(MappingLike()) == {} with pytest.raises(TypeError): - m.roundtrip_std_map_str_int(FakeMappingLike(a=1, b=2, c=3)) + m.roundtrip_std_map_str_int(FakeMappingLike(**a1b2c3)) def test_set_caster_protocol(doc): From e06dc33e5f4f28936870eb9b1fefcfcbb7079dfc Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Sun, 13 Apr 2025 23:27:16 +0200 Subject: [PATCH 11/13] Renamed and refactored sequence, mapping and set test classes to reuse implementation --- tests/test_stl.py | 72 ++++++++++++++++++----------------------------- 1 file changed, 27 insertions(+), 45 deletions(-) diff --git a/tests/test_stl.py b/tests/test_stl.py index 59d748e118..7e0c8d4b9e 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -581,7 +581,8 @@ def gen_invalid(): def test_sequence_caster_protocol(doc): from collections.abc import Sequence - class SequenceLike(Sequence): + # Implements the Sequence protocol without explicitly inheriting from collections.abc.Sequence. + class BareSequenceLike: def __init__(self, *args): self.data = tuple(args) @@ -591,15 +592,10 @@ def __len__(self): def __getitem__(self, index): return self.data[index] - class FakeSequenceLike: - def __init__(self, *args): - self.data = tuple(args) - - def __len__(self): - return len(self.data) - - def __getitem__(self, index): - return self.data[index] + # Implements the Sequence protocol by reusing BareSequenceLike's implementation. + # Additionally, inherits from collections.abc.Sequence. + class FormalSequenceLike(BareSequenceLike, Sequence): + pass assert ( doc(m.roundtrip_std_vector_int) @@ -607,17 +603,18 @@ def __getitem__(self, index): ) assert m.roundtrip_std_vector_int([1, 2, 3]) == [1, 2, 3] assert m.roundtrip_std_vector_int((1, 2, 3)) == [1, 2, 3] - assert m.roundtrip_std_vector_int(SequenceLike(1, 2, 3)) == [1, 2, 3] - assert m.roundtrip_std_vector_int(FakeSequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int(FormalSequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int(BareSequenceLike(1, 2, 3)) == [1, 2, 3] assert m.roundtrip_std_vector_int([]) == [] assert m.roundtrip_std_vector_int(()) == [] - assert m.roundtrip_std_vector_int(FakeSequenceLike()) == [] + assert m.roundtrip_std_vector_int(BareSequenceLike()) == [] def test_mapping_caster_protocol(doc): from collections.abc import Mapping - class MappingLike(Mapping): + # Implements the Mapping protocol without explicitly inheriting from collections.abc.Mapping. + class BareMappingLike: def __init__(self, **kwargs): self.data = dict(kwargs) @@ -630,18 +627,10 @@ def __getitem__(self, key): def __iter__(self): yield from self.data - class FakeMappingLike: - def __init__(self, **kwargs): - self.data = dict(kwargs) - - def __len__(self): - return len(self.data) - - def __getitem__(self, key): - return self.data[key] - - def __iter__(self): - yield from self.data + # Implements the Mapping protocol by reusing BareMappingLike's implementation. + # Additionally, inherits from collections.abc.Mapping. + class FormalMappingLike(BareMappingLike, Mapping): + pass assert ( doc(m.roundtrip_std_map_str_int) @@ -649,17 +638,18 @@ def __iter__(self): ) a1b2c3 = {"a": 1, "b": 2, "c": 3} assert m.roundtrip_std_map_str_int(a1b2c3) == a1b2c3 - assert m.roundtrip_std_map_str_int(MappingLike(**a1b2c3)) == a1b2c3 + assert m.roundtrip_std_map_str_int(FormalMappingLike(**a1b2c3)) == a1b2c3 assert m.roundtrip_std_map_str_int({}) == {} - assert m.roundtrip_std_map_str_int(MappingLike()) == {} + assert m.roundtrip_std_map_str_int(FormalMappingLike()) == {} with pytest.raises(TypeError): - m.roundtrip_std_map_str_int(FakeMappingLike(**a1b2c3)) + m.roundtrip_std_map_str_int(BareMappingLike(**a1b2c3)) def test_set_caster_protocol(doc): from collections.abc import Set - class SetLike(Set): + # Implements the Set protocol without explicitly inheriting from collections.abc.Set. + class BareSetLike: def __init__(self, *args): self.data = set(args) @@ -672,26 +662,18 @@ def __contains__(self, item): def __iter__(self): yield from self.data - class FakeSetLike: - def __init__(self, *args): - self.data = set(args) - - def __len__(self): - return len(self.data) - - def __contains__(self, item): - return item in self.data - - def __iter__(self): - yield from self.data + # Implements the Set protocol by reusing BareSetLike's implementation. + # Additionally, inherits from collections.abc.Set. + class FormalSetLike(BareSetLike, Set): + pass assert ( doc(m.roundtrip_std_set_int) == "roundtrip_std_set_int(arg0: collections.abc.Set[typing.SupportsInt]) -> set[int]" ) assert m.roundtrip_std_set_int({1, 2, 3}) == {1, 2, 3} - assert m.roundtrip_std_set_int(SetLike(1, 2, 3)) == {1, 2, 3} + assert m.roundtrip_std_set_int(FormalSetLike(1, 2, 3)) == {1, 2, 3} assert m.roundtrip_std_set_int(set()) == set() - assert m.roundtrip_std_set_int(SetLike()) == set() + assert m.roundtrip_std_set_int(FormalSetLike()) == set() with pytest.raises(TypeError): - m.roundtrip_std_set_int(FakeSetLike(1, 2, 3)) + m.roundtrip_std_set_int(BareSetLike(1, 2, 3)) From ee270ab6bfd0b727cdb5930567b980d964782dfa Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Mon, 14 Apr 2025 00:02:04 +0200 Subject: [PATCH 12/13] Added tests for mapping and set casters for noconvert mode --- tests/test_stl.cpp | 8 ++++++++ tests/test_stl.py | 26 +++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 41ff72a9fd..e9fbaede30 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -651,4 +651,12 @@ TEST_SUBMODULE(stl, m) { m.def("roundtrip_std_vector_int", [](const std::vector &v) { return v; }); m.def("roundtrip_std_map_str_int", [](const std::map &m) { return m; }); m.def("roundtrip_std_set_int", [](const std::set &s) { return s; }); + m.def( + "roundtrip_std_map_str_int_noconvert", + [](const std::map &m) { return m; }, + py::arg("m").noconvert()); + m.def( + "roundtrip_std_set_int_noconvert", + [](const std::set &s) { return s; }, + py::arg("s").noconvert()); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 7e0c8d4b9e..b34208b699 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -632,17 +632,29 @@ def __iter__(self): class FormalMappingLike(BareMappingLike, Mapping): pass + a1b2c3 = {"a": 1, "b": 2, "c": 3} + # convert mode assert ( doc(m.roundtrip_std_map_str_int) == "roundtrip_std_map_str_int(arg0: collections.abc.Mapping[str, typing.SupportsInt]) -> dict[str, int]" ) - a1b2c3 = {"a": 1, "b": 2, "c": 3} assert m.roundtrip_std_map_str_int(a1b2c3) == a1b2c3 assert m.roundtrip_std_map_str_int(FormalMappingLike(**a1b2c3)) == a1b2c3 assert m.roundtrip_std_map_str_int({}) == {} assert m.roundtrip_std_map_str_int(FormalMappingLike()) == {} with pytest.raises(TypeError): m.roundtrip_std_map_str_int(BareMappingLike(**a1b2c3)) + # noconvert mode + assert ( + doc(m.roundtrip_std_map_str_int_noconvert) + == "roundtrip_std_map_str_int_noconvert(m: dict[str, int]) -> dict[str, int]" + ) + assert m.roundtrip_std_map_str_int_noconvert(a1b2c3) == a1b2c3 + assert m.roundtrip_std_map_str_int_noconvert({}) == {} + with pytest.raises(TypeError): + m.roundtrip_std_map_str_int_noconvert(FormalMappingLike(**a1b2c3)) + with pytest.raises(TypeError): + m.roundtrip_std_map_str_int_noconvert(BareMappingLike(**a1b2c3)) def test_set_caster_protocol(doc): @@ -667,6 +679,7 @@ def __iter__(self): class FormalSetLike(BareSetLike, Set): pass + # convert mode assert ( doc(m.roundtrip_std_set_int) == "roundtrip_std_set_int(arg0: collections.abc.Set[typing.SupportsInt]) -> set[int]" @@ -677,3 +690,14 @@ class FormalSetLike(BareSetLike, Set): assert m.roundtrip_std_set_int(FormalSetLike()) == set() with pytest.raises(TypeError): m.roundtrip_std_set_int(BareSetLike(1, 2, 3)) + # noconvert mode + assert ( + doc(m.roundtrip_std_set_int_noconvert) + == "roundtrip_std_set_int_noconvert(s: set[int]) -> set[int]" + ) + assert m.roundtrip_std_set_int_noconvert({1, 2, 3}) == {1, 2, 3} + assert m.roundtrip_std_set_int_noconvert(set()) == set() + with pytest.raises(TypeError): + m.roundtrip_std_set_int_noconvert(FormalSetLike(1, 2, 3)) + with pytest.raises(TypeError): + m.roundtrip_std_set_int_noconvert(BareSetLike(1, 2, 3)) From db17f2de1db7100548fb71047e6acec112efe1ae Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Mon, 14 Apr 2025 00:15:54 +0200 Subject: [PATCH 13/13] Added tests for sequence caster for noconvert mode --- tests/test_stl.cpp | 4 ++++ tests/test_stl.py | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index e9fbaede30..5eff4f5838 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -651,6 +651,10 @@ TEST_SUBMODULE(stl, m) { m.def("roundtrip_std_vector_int", [](const std::vector &v) { return v; }); m.def("roundtrip_std_map_str_int", [](const std::map &m) { return m; }); m.def("roundtrip_std_set_int", [](const std::set &s) { return s; }); + m.def( + "roundtrip_std_vector_int_noconvert", + [](const std::vector &v) { return v; }, + py::arg("v").noconvert()); m.def( "roundtrip_std_map_str_int_noconvert", [](const std::map &m) { return m; }, diff --git a/tests/test_stl.py b/tests/test_stl.py index b34208b699..96c84374ce 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -597,6 +597,7 @@ def __getitem__(self, index): class FormalSequenceLike(BareSequenceLike, Sequence): pass + # convert mode assert ( doc(m.roundtrip_std_vector_int) == "roundtrip_std_vector_int(arg0: collections.abc.Sequence[typing.SupportsInt]) -> list[int]" @@ -608,6 +609,22 @@ class FormalSequenceLike(BareSequenceLike, Sequence): assert m.roundtrip_std_vector_int([]) == [] assert m.roundtrip_std_vector_int(()) == [] assert m.roundtrip_std_vector_int(BareSequenceLike()) == [] + # noconvert mode + assert ( + doc(m.roundtrip_std_vector_int_noconvert) + == "roundtrip_std_vector_int_noconvert(v: list[int]) -> list[int]" + ) + assert m.roundtrip_std_vector_int_noconvert([1, 2, 3]) == [1, 2, 3] + assert m.roundtrip_std_vector_int_noconvert((1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int_noconvert(FormalSequenceLike(1, 2, 3)) == [ + 1, + 2, + 3, + ] + assert m.roundtrip_std_vector_int_noconvert(BareSequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int_noconvert([]) == [] + assert m.roundtrip_std_vector_int_noconvert(()) == [] + assert m.roundtrip_std_vector_int_noconvert(BareSequenceLike()) == [] def test_mapping_caster_protocol(doc):