Updated STL casters and py::buffer to use collections.abc (#5566)

* Updated STL type hints use support collections.abc

* Updated array_caster to match numpy/eigen typing.Annotated stlye

* Added support for Mapping, Set and Sequence derived from collections.abc.

* Fixed merge of typing.SupportsInt in new tests

* Integrated collections.abc checks into convertible check functions.

* Changed type hint of py::buffer to collections.abc.Buffer

* Changed convertible check function names

* Added comments to convertible check functions

* Removed checks for methods that are already required by the abstract base class

* Improved mapping caster test using more compact a1b2c3 variable

* Renamed and refactored sequence, mapping and set test classes to reuse implementation

* Added tests for mapping and set casters for noconvert mode

* Added tests for sequence caster for noconvert mode

Author: timohl

include/pybind11/cast.h

@@ -1328,7 +1328,7 @@ struct handle_type_name<bytes> {
 };
 template <>
 struct handle_type_name<buffer> {
-    static constexpr auto name = const_name("Buffer");
+    static constexpr auto name = const_name("collections.abc.Buffer");
 };
 template <>
 struct handle_type_name<int_> {

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).
 
@@ -69,10 +69,13 @@ 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 PyObjectIsInstanceWithOneOfTpNames(PyObject *obj,
-                                               std::initializer_list<const char *> tp_names) {
+inline bool object_is_instance_with_one_of_tp_names(PyObject *obj,
+                                                    std::initializer_list<const char *> tp_names) {
     if (PyType_Check(obj)) {
         return false;
     }
@@ -85,37 +88,48 @@ 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 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());
     }
-    return (PyGen_Check(obj) != 0) || (PyAnySet_Check(obj) != 0)
-           || PyObjectIsInstanceWithOneOfTpNames(
-               obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"});
+    // 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 PyObjectTypeIsConvertibleToStdSet(PyObject *obj) {
-    return (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"});
+inline bool object_is_convertible_to_std_set(const handle &src, bool convert) {
+    // 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")));
 }
 
-inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) {
-    if (PyDict_Check(obj)) {
+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
-    if (PyMapping_Check(obj) == 0) {
-        return false;
-    }
-    PyObject *items = PyObject_GetAttrString(obj, "items");
-    if (items == nullptr) {
-        PyErr_Clear();
-        return false;
+    // 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
+    // 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) {
+            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;
+    // In convert mode: Allow types derived from collections.abc.Mapping
+    return convert && isinstance(src, module_::import("collections.abc").attr("Mapping"));
 }
 
 //
@@ -172,7 +186,7 @@ struct set_caster {
 
 public:
     bool load(handle src, bool convert) {
-        if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) {
+        if (!object_is_convertible_to_std_set(src, convert)) {
             return false;
         }
         if (isinstance<anyset>(src)) {
@@ -203,7 +217,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 <typename Type, typename Key, typename Value>
@@ -234,7 +250,7 @@ struct map_caster {
 
 public:
     bool load(handle src, bool convert) {
-        if (!PyObjectTypeIsConvertibleToStdMap(src.ptr())) {
+        if (!object_is_convertible_to_std_map(src, convert)) {
             return false;
         }
         if (isinstance<dict>(src)) {
@@ -274,7 +290,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("]"));
 };
 
@@ -283,7 +300,7 @@ struct list_caster {
     using value_conv = make_caster<Value>;
 
     bool load(handle src, bool convert) {
-        if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) {
+        if (!object_is_convertible_to_std_vector(src)) {
             return false;
         }
         if (isinstance<sequence>(src)) {
@@ -340,7 +357,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 <typename Type, typename Alloc>
@@ -416,7 +435,7 @@ struct array_caster {
 
 public:
     bool load(handle src, bool convert) {
-        if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) {
+        if (!object_is_convertible_to_std_vector(src)) {
             return false;
         }
         if (isinstance<sequence>(src)) {
@@ -474,10 +493,12 @@ struct array_caster {
     using cast_op_type = movable_cast_op_type<T_>;
 
     static constexpr auto name
-        = const_name<Resizable>(const_name(""), const_name("Annotated[")) + const_name("list[")
-          + value_conv::name + const_name("]")
-          + const_name<Resizable>(
-              const_name(""), const_name(", FixedSize(") + const_name<Size>() + const_name(")]"));
+        = const_name<Resizable>(const_name(""), const_name("typing.Annotated["))
+          + io_name("collections.abc.Sequence", "list") + const_name("[") + value_conv::name
+          + const_name("]")
+          + const_name<Resizable>(const_name(""),
+                                  const_name(", \"FixedSize(") + const_name<Size>()
+                                      + const_name(")\"]"));
 };
 
 template <typename Type, size_t Size>

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"
     )
 
 

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)

tests/test_pytypes.py

@@ -1254,7 +1254,7 @@ def test_arg_return_type_hints(doc):
     # std::vector<T>
     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<T, T>
     assert (

tests/test_stl.cpp

@@ -648,4 +648,19 @@ TEST_SUBMODULE(stl, m) {
         }
         return zum;
     });
+    m.def("roundtrip_std_vector_int", [](const std::vector<int> &v) { return v; });
+    m.def("roundtrip_std_map_str_int", [](const std::map<std::string, int> &m) { return m; });
+    m.def("roundtrip_std_set_int", [](const std::set<int> &s) { return s; });
+    m.def(
+        "roundtrip_std_vector_int_noconvert",
+        [](const std::vector<int> &v) { return v; },
+        py::arg("v").noconvert());
+    m.def(
+        "roundtrip_std_map_str_int_noconvert",
+        [](const std::map<std::string, int> &m) { return m; },
+        py::arg("m").noconvert());
+    m.def(
+        "roundtrip_std_set_int_noconvert",
+        [](const std::set<int> &s) { return s; },
+        py::arg("s").noconvert());
 }