[BUG]: stl_bind generates invalid type signature for Sphinx/Pylance

[Skylion007]

Required prerequisites

• Make sure you've read the documentation. Your issue may be addressed there.
• Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
• Consider asking first in the Gitter chat room or in a Discussion.

Problem description

STL_bind creates subclasses that try to mimic keyview. However, our subclasses have no explicit relation with typing.collections.KeyView or collection.abc.KeyView etc... #3985 attempts to fix it by removing the [], but this seems like a cludge and we really should let the typing system represent these as the correct type. I am wondering if there is a way we can still have Sphinx / Pylance recognize these KeyView, ItemViews etc as what they are instead of just an opaque class. Maybe just having them inherit from / mixin collections.abc.KeyViews would be appropiate?

Would appreciate thoughts on this front @rwgk @henryiii?

Reproducible example code

No response

[Skylion007]

@bmerry Since you created the original PR for this feature, any thoughts on if there is a clever way to solve this issue?

[bmerry]

@bmerry Since you created the original PR for this feature, any thoughts on if there is a clever way to solve this issue?

No, no clever ideas. I don't think I've touched pybind11 since I wrote that code, and I think the square bracket convention predated the changes I made.

[odel4y]

The current naming with e.g. KeysView[BindedMap] also prevents formatting the stubs with black, because it stumbles on the IMO incorrect Python syntax. Is there a reason against a name with valid syntax such as KeysViewBindedMap?

I guess the reason to inherit the class from e.g. collections.abc.KeysView would be to allow checking its capability with something like this:

isinstance(keys_view_binded_map, collections.abc.KeysView)

If so, could this be done with creating a Python class like #1193 (comment) or specifically indicating the base class like #1193 (comment) ?

[Skylion007]

I actually figured that there is a typing.KeysView etc.. that we can use. However, we need to specify the keytype, value_type.

Whenever I insert the following line into stl_bind.h, it gives a linker error though for Clang:

        constexpr const char *keyname = detail::make_caster<KeyType>::name.text;

error:

ld: illegal text reloc in '__ZN8pybind118bind_mapINSt3__113unordered_mapINS1_12basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEE9LocalBaseILi0EENS1_4hashIS8_EENS1_8equal_toIS8_EENS6_INS1_4pairIKS8_SA_EEEEEENS1_10unique_ptrISJ_NS1_14default_deleteISJ_EEEEJEEENS_6class_IT_JT0_EEENS_6handleERSG_DpOT1_' to '__ZN8pybind116detail13string_casterINSt3__112basic_stringIcNS2_11char_traitsIcEENS2_9allocatorIcEEEELb0EE4nameE' for architecture x86_64

Thoughts @rwgk @henryiii on how to fix this? It seems like the culprit its converting the descr compiletime string into a runtime one.

[Skylion007]

I found a workaround to the linker error, but if I fix the typing system, we get an error because we define the same classname multiple times:

diff:

diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h
index 22a29b47..bc1e0312 100644
--- a/include/pybind11/stl_bind.h
+++ b/include/pybind11/stl_bind.h
@@ -651,6 +651,15 @@ struct items_view {
     Map ↦
 };

+/*
+// Helper function needed to prevent linker error
+template <typename Value>
+const char* get_caster_name(){
+       static constexpr auto value_name = detail::make_caster<Value>::name;
+       return value_name.text;
+}
+*/
+
 PYBIND11_NAMESPACE_END(detail)

 template <typename Map, typename holder_type = std::unique_ptr<Map>, typename... Args>
@@ -662,6 +671,12 @@ class_<Map, holder_type> bind_map(handle scope, const std::string &name, Args &&
     using ItemsView = detail::items_view<Map>;
     using Class_ = class_<Map, holder_type>;

+       // Need to do an explicit copy from static to local to prevent linker errors
+       static constexpr auto key_conv_name = detail::make_caster<KeyType>::name;
+       static constexpr auto value_conv_name = detail::make_caster<MappedType>::name;
+       const char *key_name = key_conv_name.text;
+       const char *value_name = value_conv_name.text;
+
     // If either type is a non-module-local bound type then make the map binding non-local as well;
     // otherwise (e.g. both types are either module-local or converting) the map will be
     // module-local.
@@ -674,11 +689,11 @@ class_<Map, holder_type> bind_map(handle scope, const std::string &name, Args &&

     Class_ cl(scope, name.c_str(), pybind11::module_local(local), std::forward<Args>(args)...);
     class_<KeysView> keys_view(
-        scope, ("KeysView[" + name + "]").c_str(), pybind11::module_local(local));
+        scope, (std::string("KeysView[") + key_name + ']').c_str(), pybind11::module_local(local));
     class_<ValuesView> values_view(
-        scope, ("ValuesView[" + name + "]").c_str(), pybind11::module_local(local));
+        scope, (std::string("ValuesView[") + value_name + ']').c_str(), pybind11::module_local(local));
     class_<ItemsView> items_view(
-        scope, ("ItemsView[" + name + "]").c_str(), pybind11::module_local(local));
+        scope, (std::string("ItemsView[") + key_name + ',' + value_name + ']').c_str(), pybind11::module_local(local));

     cl.def(init<>());

Error:

    import pybind11_tests
E   ImportError: generic_type: cannot initialize type "KeysView[str]": an object with that name is already defined

[virtuald]

Is the name used for anything other than rendering documentation? If not, then I don't see why we can't allow duplicates. If so, then add the ability to specify both an 'actual' internal name and a doc-only name used for rendering docs.