Use annotated for array

[cielavenir]

Description

Addressed #3916 (comment) Fixes #3912

Suggested changelog entry:

* Fixes docstring generation for ``std::array``-list caster. Previously, signatures included the size of the list in a non-standard, non-spec compliant way. The new format conforms to PEP 593. **Tooling for processing the docstrings may need to be updated accordingly.**

(oh I copy-pasted 3916)

---

/cc @Skylion007
/cc @felixvd

[rwgk]

Hi @cielavenir did you already see the AppVeyor failure? It looks like a couple more tests need updating. I'm holding off triggering the rest of the GitHub Actions until you get a chance to fix. I believe without a fix there will be so many failures that we're not learning much from the full results.

[cielavenir]

@rwgk thank you, fixed

[cielavenir]

oh why valarray is array_caster not list_caster? hopefully fixed though

[rwgk]

(I triggered a rerun to get rid of a notorious flake.)

[rwgk]

I ran this by my team mates (includes pytype developers)

they didn't like the plain 2

so I looked here: https://docs.python.org/3/library/typing.html#typing.Annotated

where I see e.g. Annotated[list[tuple[int, int]], MaxLen(10)]

based on that I suggested Annotated[List[int], FixedSize(2)]

which they liked but pointed out that FixedSize needs to be defined somewhere:

$ python3.11
>>> from typing import Annotated
>>> def f() -> Annotated[list[int], FixedSize(2)]: ...
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'FixedSize' is not defined

I'm totally out of my depth ... how does that idea play with pybind11-stubgen?

[cielavenir]

It works with future

In [1]: from __future__ import annotations

In [2]: from typing import Annotated

In [3]: def f() -> Annotated[list[int], FixedSize(2)]: pass

(but still, let me check mypy output)

[cielavenir]

@rwgk oh, mypy works (after another patch of pybind11-stubgen) but pyright shows this __init__.pyi:182:68 - error: "FixedSize" is not defined (reportUndefinedVariable)

[cielavenir]

in Python3 typing documentation, ValueRange and MaxLen are not defined - how they write sample code in the way pyright does not understand?

[cielavenir]

And... do you mean this stackoverflow post is not ok?

https://stackoverflow.com/questions/44833822/specify-length-of-sequence-or-list-with-python-typing-module/67146944#67146944

[felixvd]

As mentioned in this SO post, the MaxLen(10) example in the Annotated doc is not built in, but an arbitrary function.

It would be great if there were defaults in typing_extensions, but leaving this up to the community seems to have been intentional. Defining the function like this should not be a problem:

from typing import TYPE_CHECKING
if TYPE_CHECKING:
    def FixedSize(value):
        pass  # Type hint annotation for containers of fixed length

I note that both Annotated[List[str], 2] and Annotated[List[str], FixedSize(2)] are an improvement over the current incorrect syntax List[str[2]].

[cielavenir]

@felixvd pybind11-stubgen update: https://github.com/cielavenir/pybind11-stubgen/commits/addFixedSize

[rwgk]

@felixvd pybind11-stubgen update: https://github.com/cielavenir/pybind11-stubgen/commits/addFixedSize

Looks great to me! (purely from a readability pov; I know very little about typing)

I want to run this through global testing (Google-internal toolchain) after you had a chance to fix up the tests.

[rwgk]

Nice, all green! I'll run this through global testing tonight.

[rwgk]

There is one failing test in the global testing, exactly what I was afraid would happen. It's in the non-public part of our codebase, which is good news in the sense that I can change that code together with the import of this PR. Bad news for me because I'll probably have to spend at least a couple hours to negotiate with the owning team and see it through.

AFAIU there is a tool that harvests the pybind11 docstrings to create typestubs of some sorts. They have a command to update the typestubs automatically, the result is akin to a commit to be merged. Currently it looks like this:

-    def world_origin_world_to_camera(self) -> List[float[3]]: ...
+    def world_origin_world_to_camera(self) -> Any: ...

It's not a surprise to see a change, but apparently the tool doesn't know at all how to handle Annotated[List[float], FixedSize(3)] and simply falls back to Any.

As I hinted, I can work with the team owning the typestub generator, but in the bigger picture, the rest of the world might have similar problems.

Just pointing out, I think this is a good change and getting the world to adjust is an OK cost.

@cielavenir & @felixvd, could you please formally approve this PR if you agree? I'll merge this when I see both approvals.

---

Internal reference: OCL:534690556:BASE:534836523:1684939647732:8df15d67 (to help me find it again later)

[cielavenir]

@rwgk pull request author cannot approve his own changeset, but I'm fine if I don't need to perform further changes on pybind11.

a tool that harvests the pybind11 docstrings to create typestubs of some sorts

interesting if other than pybind11-stubgen, though I will have to ask pybind11-stubgen staff to merge https://github.com/cielavenir/pybind11-stubgen/commits/addFixedSize anyway

[felixvd]

As you said, this is a change that tools may need to adjust to. But since it follows PEP 593 and I believe is an improvement over the previous custom syntax which predates the PEP, I agree that the cost is worth it.

Thank you for the quick review and testing. Much appreciated.

It might be a good idea to loop in the team owning the typestub generation tool so they can comment before merging.

[rwgk]

It might be a good idea to loop in the team owning the typestub generation tool so they can comment before merging.

I reached out already; waiting to hear back. We're not deploying directly from master, I'll go ahead and merge here. Thanks all!

[rwgk]

@cielavenir & @felixvd Do you have experience working on mypy stubgen, or know someone who could make the required changes there?

It might be a good idea to loop in the team owning the typestub generation tool so they can comment before merging.

I reached out already; waiting to hear back.

We converged on this idea (full context but the only point that matters here is 4.):

1. disable the failing typestub test
2. manually update the checked-in .pyi files
3. import the latest pybind11 (into the Google codebase)
4. update mypy stubgen on github & import (into Google codebase)
5. re-enable the typestub test

[cielavenir]

if other than pybind11-stubgen

@rwgk So you mean https://github.com/python/mypy/blob/master/mypy/stubgenc.py one right? I was not aware of that module (I was aware of mypy itself though).

[rwgk]

if other than pybind11-stubgen

@rwgk So you mean https://github.com/python/mypy/blob/master/mypy/stubgenc.py one right? I was not aware of that module (I was aware of mypy itself though).

I think the answer is yes. What I know for sure is that we're using

https://github.com/python/mypy/blob/master/mypy/stubgen.py

This version exactly, which calls stubgenc:

https://github.com/python/mypy/blob/b5914b51304800e82d435310e548547d2b55298c/mypy/stubgen.py#L110

[cielavenir]

• pybind11-stubgen Add fixed size sizmailov/pybind11-stubgen#99
• mypy Fix pybind stubs20230526 python/mypy#15308