pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.18k stars 2.06k forks source link

fix: Use lowercase builtin collection names #4833

Closed sizmailov closed 10 months ago

sizmailov commented 10 months ago

Description

The #4259 introduced lowercase annotations for collections (e.g. list for pybind::typing::List), while the other parts of the pybind11 still use uppercase annotations for the same python types. This might lead to a mix of upper/lower case for collections:

    m.def("mix", [](const std::array<int, 3> &, const py::typing::List<int>&) {});
print(demo.mix.__doc__)
# mix(arg0: Annotated[List[int], FixedSize(3)], arg1: list[int]) -> None
#                     ^^^^                            ^^^^

This PR fixes this inconsistency, rendering all collection types in lowercase according to PEP 585.

Note: The change is NOT incompatible with Python 3.6, since it alters only docstrings.

Fixes #4828

Suggested changelog entry:

Builtins collections names in docstrings are now consistently rendered in lowercase (list, set, dict, tuple), in accordance with PEP 585.
sizmailov commented 10 months ago

@rwgk Is it better now?

rwgk commented 10 months ago

Yes, thanks, looks great! The only thing I'd still change is the changelog entry, maybe:

Builtins collections names in docstrings are now consistently rendered in lowercase (list, set, dict, tuple), in accordance with PEP 585.

sizmailov commented 10 months ago

@rwgk I've updated the changelog entry. Thanks for the suggestion.

Skylion007 commented 10 months ago

This feels like it warrants a version bump as it will break docstring unit tests.

rwgk commented 10 months ago

This feels like it warrants a version bump

That was my assumption (pybind11 v2.12). We already have an internals version bump for MSVC on master.

as it will break docstring unit tests.

Could you please explain more? I'm not sure.