Description
Bug Report
We're experimenting under pybind/pybind11#4888 with a pybind11 behavior change, to generate
Annotated[Any, CppTypePybind11("cpp_namespace::UserType")]
instead of a bare cpp_namespace::UserType
.
It seems like mypy does handle outer-level Annotated
, but not nested ones, e.g.
Iterator[str, Annotated[Any, CppTypePybind11("cpp_namespace::UserType")]]
The results from stubgen (1.5.1, Google-internal toolchain) are like this:
def values(self, *args, **kwargs) -> Any: ...
Desired is:
def values(self) -> Iterator[str, Annotated[Any, CppTypePybind11("cpp_namespace::UserType")]]: ...
Does this ring any bells?
If you need more details, please let me know. (Note though that I don't know a lot about typing and mypy. This fell into my lap as Google-internal pybind11 owner & pybind11 maintainer on github.)
A more general question: What are your thought about changing the pybind11 behavior in this way? Is there a better way?
(A clear and concise description of what the bug is.)
To Reproduce
# Ideally, a small sample program that demonstrates the problem.
# Or even better, a reproducible playground link https://mypy-play.net/ (use the "Gist" button)
Expected Behavior
Actual Behavior
Your Environment
- Mypy version used: 1.5.1, Google-internal toolchain
- Mypy command-line flags:
- Mypy configuration options from
mypy.ini
(and other config files): - Python version used: 3.10
Activity
Annotated[Any, pybind11.CppType("cpp_namespace::UserType")]
pybind/pybind11#4888[-]stubgen & `Annotated[Any, CppTypePybind11("cpp_namespace::UserType")]`[/-][+]stubgen & pybind11: can we make them play nice(r)?[/+]sizmailov commentedon Nov 11, 2023
@JelleZijlstra @chadrik
Could you please take a look at the issue?
chadrik commentedon Nov 11, 2023
Happy to help.
I saw that my new changes to
stubgen
were released in mypy 1.7, and those changes included some improvements to parsing recursive types.@rwgk If possible, it would be great if you could test mypy 1.7 as there is a chance this problem is fixed. If it's difficult to get updated tooling installed inside Google, that's ok, my suggestion below should be fine on its own:
The fastest way to resolve this will be for you to modify the mypy pybind11 demo source code to demonstrate your problem and make a PR with the changes. I'll then be able to much more easily improve the stubgen code to resolve the issue.
rwgk commentedon Nov 13, 2023
Trying. (It's not easy; I have to run global testing and fix anything that breaks. Might take a while.)
I could add to that file, but asking first: Are you set up to test in combination with an open pybind11 PR (pybind/pybind11#4888)?
Or alternatively, maybe better: Could you point me to the commands I'd need to run to test myself, in my fork of mypy?
chadrik commentedon Nov 13, 2023
No. But maybe there's an even simpler way to cooperate on this. Can you provide here the exact docstring that would be inspected from a function with this new capability? I can write a simpler unit test just focused on the string parsing.
chadrik commentedon Nov 13, 2023
I should add that if the annotation in the docstring parser could be tripped up by use of parentheses in the types:
Iterator[str, Annotated[Any, CppTypePybind11("cpp_namespace::UserType")]]
. Is this something that you expect mypy or other type analyzers to understand? In order for this to work we would also need to generate an import statement forCppTypePybind11
, I assume.rwgk commentedon Nov 13, 2023
I tried to get a docstring for bindings in the wild, but no luck in the time I can give it right now. I'll try again asap.
I don't know a lot about typing. My main idea is that we change pybind11 so that mypy is not tripped up by a raw C++ name (e.g.
cpp_namespace::UserType
). I don't know (at all) what exactly is best. I'll go with your advice.Would this be better?
bluenote10 commentedon Nov 13, 2023
Why is it nested to begin with?
Iterator
only takes one generic argument, soIterator[str, Annotated[Any, "cpp_namespace::UserType"]]
doesn't seem to make sense as a type.Annotated[Iterator[str], "cpp_namespace::UserType"]
would.chadrik commentedon Nov 13, 2023
Annotated[Any, "cpp_namespace::UserType"]
is preferable toAnnotated[Any, CppTypePybind11("cpp_namespace::UserType")]
unless there is an actual importable python type calledCppTypePybind11
, in which case you should include the module name and the new version of stubgen in mypy 1.7 should create the import statement. e.g.Annotated[Any, pybind11.CppTypePybind11("cpp_namespace::UserType")]
chadrik commentedon Nov 13, 2023
With regard to nesting,
Annotated
should wrap the type that representscpp_namespace::UserType
. @bluenote10 is correct thatIterator
only takes one argument.So, if the
Any
represents the UserType, then this would be correct:Or if the iterator of strings represents the UserType, this would be correct:
Or if the intent was that it's an iterator of a tuple of
(str, Any)
, where theAny
is the UserType, then this would be correct:rwgk commentedon Nov 13, 2023
Thanks! That's exactly what I needed to know.
I'll work on constructing pybind11 unit tests (in my pybind11 PR), then share the docstrings here.
sizmailov commentedon Nov 14, 2023
It would be a bug in the parser, then. It's not uncommon to have arbitrary expressions in
Annotated[...]
Note that pybind11 already uses
Annotated
for fixed-sized arrays (pybind/pybind11#4679):std::array<int, 2>
is rendered asAnnotated[List[int], FixedSize(2)]
. (not sure if stubgen 1.7 is ok with that)Bare-string annotations are less clear and always would be troublesome for a third-party tool that works with annotations. I agree, that having a valid importable class is even better.
The following annotation for a raw C++ type should be perfect.
17 remaining items