-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG]: stl_bind generates invalid type signature for Sphinx/Pylance #3986
Comments
@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. |
The current naming with e.g. I guess the reason to inherit the class from e.g. 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) ? |
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:
error:
Thoughts @rwgk @henryiii on how to fix this? It seems like the culprit its converting the descr compiletime string into a runtime one. |
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:
Error:
|
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. |
Required prerequisites
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
The text was updated successfully, but these errors were encountered: