fix(types): provide better type hints for a variety of generic types #4259

virtuald · 2022-10-19T23:47:02Z

Description

Makes for better documentation
Added support for annotating tuple, dict, list, set, function

@sizmailov I'm pretty sure this will allow users to make better .pyi files too. :)

Note: I'm open to bikeshedding on the names, but adding an underscore seemed like a reasonable approach.

Suggested changelog entry:

Special 'typed' wrappers now available in typing.h to annotate tuple, dict, list, set, and function

Skylion007

Instead of wrappers, is there anyway we can abuse the existing C++ classes? Like having an optional template arg that has a default value of "Any" or "object". Then if that special template arg is specified we revert to the old caster?

virtuald · 2022-10-20T00:14:32Z

I think that would work.... though, would it break users that are using multiple modules with different versions of pybind11?

Skylion007 · 2022-10-20T00:15:46Z

Or if we want to go with legacy Python names, just copy the capitalized nature of the collections Tuple, Set...

virtuald · 2022-10-20T01:03:42Z

I like that better, and it matches the annotation as well. I also changed function_ to Callable for consistency.

EthanSteinberg · 2022-10-20T13:46:51Z

If you are trying to support stub generation, you should probably add some unit tests for that stub generation (but in a separate PR).

Skylion007 · 2022-10-20T14:50:13Z

Thinking about this a bit more, certain casters already STRICTLY define and check/these types when relevant and these annotations won't be runtime checks (nor should they for performance reasons), but i worry that this may be confusing to a user who would be surprised when a particular cast breaks. @virtuald Also, some more information could specified with typing.Annotated, do we have know how we would support that in the future? We probably should also move some of our casters like the Array Caster to that see #3916 for example.

virtuald · 2022-10-20T18:34:43Z

@Skylion007

i worry that this may be confusing to a user who would be surprised when a particular cast breaks

Then let's put a note in there that it's just for documentation purposes? It's python, we're never guaranteed that anything gets validated at runtime.

Also, some more information could specified with typing.Annotated, do we have know how we would support that in the future?

I'm less interested in mypy validation and more interested in documentation and better IDE support. Historically IDEs haven't tried to do introspection on compiled packages, so things like autocomplete and documentation wouldn't work. In that spirit, if there are things we can add that make pybind11-stubgen's life easier, then that makes a lot of sense to me... but other things seem less useful. With a .pyi file next to my compiled library, when I click on the function in my code in my properly configured IDE, it brings me right to the pyi documentation, and things like autocomplete and types work as expected. I think mypy will use the pyi file if present as well.

@lalaland I did add tests for stub generation? pybind11-stubgen generates stubs from the docstrings that pybind11 generates, and the test I added ensures they're in the right form. Unless you're going to integrate pybind11-stubgen into pybind11, there's nothing more to do here.

Skylion007 · 2022-10-20T21:02:52Z

@virtuald What about the edge case where a user wants to specify Tuple for consistency? IE. no template args.

virtuald · 2022-10-20T21:31:33Z

Added that.

I note that the capitalized form of the builtin types (eg, Tuple) is deprecated as of python 3.9, so we could conditionally change the caseness based on the python version. Thinking about it, I think maybe since this is just a docstring, that we should leave the desired form up to the stub generator. It'll also be consistent with our other casters that have the uppercase form.

Skylion007 · 2022-10-20T22:02:34Z

What about untyped List, Set, etc...

virtuald · 2022-10-20T22:49:19Z

You have to use the lowercase version for those.

virtuald · 2022-10-21T02:38:15Z

I was contemplating a way to add an arbitrary annotation to types (like py::annotate<int, "typing.Annotate[int, (2, 3)]">, but it seems that you can't make a string a template parameter. These changes are probably fine for now though.

EthanSteinberg · 2022-10-21T02:45:54Z

@virtuald You can make a string a template parameter in C++ now

https://ctrpeach.io/posts/cpp20-string-literal-template-parameters/

Simply add some #include guards to only enable this for new enough compilers

virtuald · 2022-10-21T03:16:06Z

Ha, neat:

template <typename T, detail::descr Name>
struct Annotate : T {
    using T::T;
};

template <typename T, descr Name>
struct handle_type_name<Annotate<T, Name>> {
    static constexpr auto name = Name;
};

...

m.def("annotate_int", [](const py::Annotate<py::int_, py::detail::const_name("Annotate[int, (1,2)]")> &) {});

That feels like a separate PR though, there's lots of room for bikeshedding and depending on how one feels about const_name could be more invasive.

Skylion007 · 2022-10-21T03:16:34Z

You have to use the lowercase version for those.

Then we should have a static_assert to enforce that at least one template is specified. Otherwise, people are going to accidentally create List[] and other invalid stubs.

virtuald · 2022-10-21T03:18:49Z

@Skylion007 Only tuple has variable templates, everything else has a required template arg, no need for static assert:

template <typename T>
class List : public list {
    using list::list;
};

I don't think python type hints allow per-element hints for lists, dicts, sets.

sizmailov · 2022-10-21T03:44:07Z

@virtuald I like this idea.
It feels like all "typed" types could be a third-party library or could be defined in a separate header within pybind (e.g. typing.h).
I don't have a strong preference for one over the other; just sharing a thought.

Skylion007 · 2022-10-21T03:55:32Z

@Skylion007 Only tuple has variable templates, everything else has a required template arg, no need for static assert:
template <typename T>
class List : public list {
    using list::list;
};
I don't think python type hints allow per-element hints for lists, dicts, sets.

@virtuald , Ah typo, I meant handling the template args for Callable.

virtuald · 2022-10-21T05:05:30Z

It feels like all "typed" types could be a third-party library or could be defined in a separate header within pybind (e.g. typing.h).

I think having at least the basic python types in pybind11 makes sense, a third party library would be harder to discover. I'm.. +0.5 on putting all of this in typing.h if there are no objections... though, there are a lot of headers in pybind11 these days.

EthanSteinberg · 2022-10-21T15:11:03Z

@virtuald Why can't annotations be part of pybind11::arg?

a.def("foo_function", [](py::list name){}, py::arg("name", "List[str]") ;

py::annotate is going to be extremely confusing.

virtuald · 2022-10-21T17:18:29Z

@lalaland that's a neat approach, I like that we could specify arbitrarily complex type hints. It would have to also include a way to change the hint for return values and properties as well. Better than py::annotate.

In theory that approach could replace this current PR. However, I do like having the ability to do using MyCb = py::Callable<void(int)> and reuse it in several places -- so I think they're complementary.

EthanSteinberg · 2022-10-21T17:25:16Z

so I think they're complementary

Yep. Totally agree. I think the goal should be that pybind11 should automatically generate the correct types from the C++ argument types, with manual annotations in py::arg only as a fallback solution for when the automatic approach fails.

Skylion007 · 2022-10-23T16:11:53Z

@virtuald Could this be extended to solve this typing issue too? #3986 (comment)

Skylion007 · 2022-11-29T18:58:54Z

@virtuald, Actually, this should just be automatic for newer versions of Python for return types, right? Something to consider if we decide to refactor casters.

virtuald · 2022-12-25T06:18:02Z

I moved the typing related classes to typing.h.

I've been using this for awhile with no ill effects as far as I can tell. I don't believe there are any further changes to be made to this specific PR -- other suggestions mentioned above are better suited for their own PRs.

rwgk · 2023-07-19T19:45:44Z

I'll run this through global testing, to see if there are any surprises. (That will take 6+ hours.)

rwgk · 2023-07-20T17:00:26Z

Global testing passed (the internal test id is OCL:549398140:BASE:549657321:1689871259083:7fa91c99)

I'll review asap.

rwgk · 2023-07-20T17:15:53Z

include/pybind11/typing.h

+    There is no additional enforcement of types at runtime.
+*/
+
+template <typename... Types>


I think this feature will be clearer as such by having the types below in a typing sub-namespace.

I'm not sure about (e.g.) py::typing::Tuple vs py::typing::tuple. Pure Python typing.tuple does not exist (I tried with Python 3.11), therefore I'm guessing py::typing::Tuple might be less surprising.

rwgk · 2023-07-20T17:17:43Z

include/pybind11/typing.h

+};
+
+template <typename T>
+class List : public list {


Could this be template <typename... Types> like or Tuple?

Update: I just ran this by a typing expert on my team, from that I learned:

List[float, str] isn't a valid type annotation, since a list (unlike a tuple) can only have one element type. A list that contains both floats and strs can be typed as list[float | str] (or List[Union[float, str]] for Python 3.9- compatibility).

Is that too tricky to implement for this PR?

IMO, list[float | str] is fine / best, that's valid for older versions of Python as long as you only use it for typing and not at runtime.

Also, the equivalent for tuple is tuple[float | str, ...]. tuple[float, str] is a size 2 tuple with a float as the first item, and a str as the second.

rwgk · 2023-07-20T18:25:34Z

include/pybind11/typing.h

+template <typename... Types>
+struct handle_type_name<Tuple<Types...>> {
+    static constexpr auto name
+        = const_name("Tuple[") + concat(make_caster<Types>::name...) + const_name("]");


Another thing I learned from the typing expert team mate:

Would it be better to have lower-case names (tuple, dict, list, set) in the output?

It depends on what Python versions you need to support. In Python 3.10 and higher, it is indeed preferred to use the lowercase builtin names.

That makes me think #ifdefs of some sort would be best (to produce Tuple for <= 3.9 and tuple otherwise), although that adds a bit of clutter until we can drop 3.9 support.

Tying that back to my other comment, py::typing::tuple vs py::typing::Tuple:

Pure Python typing.Tuple will likely be deleted at some point in the future, although IIUC there is no timeline yet.

We cannot reuse py::tuple like pure Python reuses tuple for typing purposes.

Should we have py::typing::tuple or py::typing::Tuple? I don't know. It's on the awkward side either way.

I think I'd settle for py::typing::tuple (for any Python version), to not look old fashioned in the long run.

For Python 3.7-3.9, you still can use lower case as long as you only use them for typing. In this case, it can only be used for typing, not runtime, so please use the lowercase, un-namespaced names, no ifdefs. I've mostly removed typing.* stuff from libraries that are 3.7+.

+1

Note that we have already firmly decided to remove Python 3.6 support asap (as soon as I find a moment).

(Yes, rather assuming that in this discussion :) - besides, typing in 3.6 is pretty hard since everything has dropped it a while back)

I would add a class Union<T...> : object to implement union markers in a more general way.

If there was a py::typing namespace it would make more sense to implement such a thing, since py::Union is a bit weird.

I would add a class Union<T...> : object to implement union markers in a more general way.

If there was a py::typing namespace it would make more sense to implement such a thing, since py::Union is a bit weird.

Sounds great to me. I felt strongly already that py::typing is the way to go, now even more.

I gave it a shot, but I think adding would be out of scope for this current PR because of two messy issues that came up.

If you add a Union type then you get compilation errors when loading the argument because one would need to define a PYBIND11_OBJECT_COMMON for it... which, you could add a check function to check if the incoming PyObject is one of the arg types, but the common types also have implicit conversions which wouldn't be caught here.

Additionally this would require defining a concat_pipe version of concat which is mostly copy/paste, but it's another thing to maintain.

rwgk

This looks great to me now.

The only small doubt I still have, do we want py::typing::Tuple or py::typing::tuple?

I slightly favor py::typing::Tuple as we have right now in this PR. It might look old fashioned from a pure Python viewpoint, but in the context of C++ it's a nice way to not have surprise name collisions (between py::tuple and py::typing::tuple).

virtuald · 2023-07-23T19:01:11Z

I prefer Tuple over tuple.

rwgk · 2023-07-23T19:05:45Z

I prefer Tuple over tuple.

Cool, thanks.

@EthanSteinberg, @Skylion007, @henryiii, @sizmailov does anyone have a strong reason to prefer py::typing::tuple?

I'll wait a couple days for responses. If there are no objections, I'll merge this PR.

rwgk · 2023-07-23T19:06:26Z

(I just fixed my previous comment: should have been py::typing::tuple).

EthanSteinberg · 2023-07-23T19:24:41Z

@rwgk

does anyone have a strong reason to prefer py::typing::tuple?

I would support upper case Tuple to avoid name collisions.

rwgk · 2023-07-23T19:27:19Z

@rwgk

does anyone have a strong reason to prefer py::typing::tuple?

I would support upper case Tuple to avoid name collisions.

Perfect, thanks, that makes 3 votes for upper case Tuple (counting Dustin's), I'll go ahead merge this PR now.

rwgk · 2023-07-23T19:28:33Z

Oh, wait, @Skylion007, merging is blocked because you requested changes. Could you please look again?

Skylion007

I also prefer Tuple

rwgk · 2023-07-26T19:46:58Z

include/pybind11/typing.h

+    pybind11/typing.h: Convenience wrapper classes for basic Python types
+    with more explicit annotations.
+
+    Copyright (c) 2016 Wenzel Jakob <[email protected]>


Sorry I forgot to comment on this before, could you please either put your name & current year, or maybe:

// Copyright (c) 2023 The pybind Community.

(That's what I usually use.)

… information was copy-pasted from the git log output.

rwgk · 2023-08-04T05:17:34Z

@virtuald I took the liberty to update the copyright line: 072049a

I'll merge this as soon as I see that the CI is done, then I'll update the smart_holder branch.

* Provide better type hints for a variety of generic types * Makes better documentation * tuple, dict, list, set, function * Move to py::typing * style: pre-commit fixes * Update copyright line with correct year and actual author. The author information was copy-pasted from the git log output. --------- Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

@malfet

…#4772) * Copy clang 17 compatibility fixes from PR #4762 to a separate PR. * static py::exception<> -> static py::handle * Add `py::set_error()` but also try the suggestion of @malfet (pytorch/pytorch#106401 (review)). * clang 17 compatibility fixes (#4767) * Copy clang 17 compatibility fixes from PR #4762 to a separate PR. * Add gcc:13 C++20 * Add silkeh/clang:16-bullseye C++20 * chore(deps): update pre-commit hooks (#4770) updates: - [github.com/psf/black: 23.3.0 → 23.7.0](psf/black@23.3.0...23.7.0) - [github.com/astral-sh/ruff-pre-commit: v0.0.276 → v0.0.281](astral-sh/ruff-pre-commit@v0.0.276...v0.0.281) - [github.com/asottile/blacken-docs: 1.14.0 → 1.15.0](adamchainz/blacken-docs@1.14.0...1.15.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * docs: Remove upper bound on pybind11 in example pyproject.toml for setuptools (#4774) * docs: Remove upper bound on pybind11 in example pyproject.toml for setuptools * Update docs/compiling.rst --------- Co-authored-by: Henry Schreiner <[email protected]> * Provide better type hints for a variety of generic types (#4259) * Provide better type hints for a variety of generic types * Makes better documentation * tuple, dict, list, set, function * Move to py::typing * style: pre-commit fixes * Update copyright line with correct year and actual author. The author information was copy-pasted from the git log output. --------- Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Use `py::set_error()` everywhere possible (only one special case, in common.h). Overload `py::set_error(py::handle, py::handle)`. Change back to `static py::handle exc = ... .release();` Deprecate `py::exception<>::operator()` * Add `PYBIND11_WARNING_DISABLE` for INTEL and MSVC (and sort alphabetically). * `PYBIND11_WARNING_DISABLE_INTEL(10441)` does not work. For ICC only, falling back to the recommended `py::set_error()` to keep the testing simple. It is troublesome to add `--diag-disable=10441` specifically for test_exceptions.cpp, even that is non-ideal because it covers the entire file, not just the one line we need it for, and the value of exercising the trivial deprecated `operator()` on this one extra platform is practically zero. * Fix silly oversight. * NVHPC 23.5.0 generates deprecation warnings. They are currently not treated as errors, but falling back to using `py::set_error()` to not have to deal with that distraction. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Keto D. Zhang <[email protected]> Co-authored-by: Henry Schreiner <[email protected]> Co-authored-by: Dustin Spicuzza <[email protected]>

Skylion007 requested changes Oct 19, 2022

View reviewed changes

virtuald force-pushed the annotated-types branch 2 times, most recently from a3ed9d2 to 8633a05 Compare October 20, 2022 01:03

virtuald force-pushed the annotated-types branch from 8633a05 to 13afc03 Compare October 20, 2022 03:33

virtuald force-pushed the annotated-types branch from 90a0e08 to 7089306 Compare October 20, 2022 19:18

virtuald force-pushed the annotated-types branch from de87faf to 9acd090 Compare December 25, 2022 05:57

Merge branch 'master' into annotated-types

33f9bac

rwgk reviewed Jul 20, 2023

View reviewed changes

virtuald and others added 2 commits July 21, 2023 16:24

Move to py::typing

5f8b132

style: pre-commit fixes

1685758

rwgk approved these changes Jul 23, 2023

View reviewed changes

Merge branch 'master' into annotated-types

ccaeb52

Skylion007 approved these changes Jul 26, 2023

View reviewed changes

rwgk reviewed Jul 26, 2023

View reviewed changes

Ralf W. Grosse-Kunstleve added 3 commits August 3, 2023 22:05

Merge branch 'master' into annotated-types

f9d8694

Update copyright line with correct year and actual author. The author…

072049a

… information was copy-pasted from the git log output.

Merge branch 'master' into annotated-types

8e6b00f

rwgk merged commit f870315 into pybind:master Aug 4, 2023
82 checks passed

github-actions bot added the needs changelog Possibly needs a changelog entry label Aug 4, 2023

sizmailov mentioned this pull request Aug 31, 2023

[BUG]: Inconsistent case in annotations of python collections #4828

Closed

3 tasks

sizmailov mentioned this pull request Sep 7, 2023

fix: Use lowercase builtin collection names #4833

Merged

henryiii changed the title ~~Provide better type hints for a variety of generic types~~ fix(types) provide better type hints for a variety of generic types Nov 15, 2023

henryiii changed the title ~~fix(types) provide better type hints for a variety of generic types~~ fix(types): provide better type hints for a variety of generic types Nov 15, 2023

henryiii removed the needs changelog Possibly needs a changelog entry label Mar 27, 2024

fix(types): provide better type hints for a variety of generic types #4259

fix(types): provide better type hints for a variety of generic types #4259

Conversation

virtuald commented Oct 19, 2022 • edited Loading

Description

Suggested changelog entry:

Skylion007 left a comment

Choose a reason for hiding this comment

virtuald commented Oct 20, 2022

Skylion007 commented Oct 20, 2022

virtuald commented Oct 20, 2022

EthanSteinberg commented Oct 20, 2022 • edited Loading

Skylion007 commented Oct 20, 2022

virtuald commented Oct 20, 2022

Skylion007 commented Oct 20, 2022 • edited Loading

virtuald commented Oct 20, 2022

Skylion007 commented Oct 20, 2022

virtuald commented Oct 20, 2022

virtuald commented Oct 21, 2022

EthanSteinberg commented Oct 21, 2022

virtuald commented Oct 21, 2022

Skylion007 commented Oct 21, 2022 • edited Loading

virtuald commented Oct 21, 2022

sizmailov commented Oct 21, 2022

Skylion007 commented Oct 21, 2022

virtuald commented Oct 21, 2022

EthanSteinberg commented Oct 21, 2022 • edited Loading

virtuald commented Oct 21, 2022 • edited Loading

EthanSteinberg commented Oct 21, 2022

Skylion007 commented Oct 23, 2022

Skylion007 commented Nov 29, 2022

virtuald commented Dec 25, 2022

rwgk commented Jul 19, 2023

rwgk commented Jul 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk left a comment

Choose a reason for hiding this comment

virtuald commented Jul 23, 2023

rwgk commented Jul 23, 2023 • edited Loading

rwgk commented Jul 23, 2023

EthanSteinberg commented Jul 23, 2023

rwgk commented Jul 23, 2023

rwgk commented Jul 23, 2023

Skylion007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk commented Aug 4, 2023

virtuald commented Oct 19, 2022 •

edited

Loading

EthanSteinberg commented Oct 20, 2022 •

edited

Loading

Skylion007 commented Oct 20, 2022 •

edited

Loading

Skylion007 commented Oct 21, 2022 •

edited

Loading

EthanSteinberg commented Oct 21, 2022 •

edited

Loading

virtuald commented Oct 21, 2022 •

edited

Loading

rwgk commented Jul 23, 2023 •

edited

Loading