feat: py::type::of<T>() and py::type::of(h) by henryiii · Pull Request #2364 · pybind/pybind11

henryiii · 2020-08-04T15:07:06Z

This adds py::type::of<T>(), which provides a way to get the Python type associated with a C++ type. Currently there are two possible ways to do this (pointed out by @YannickJadoul and @bstaletic on Gitter about a year ago, IIRC), but they require usage of internal structures or the detail namespace. This provides a natural, pythonic way to get the type object without reverting to internal details.

This is very useful in templated code; for example, in boost-histogram, it is needed to get the templated storage from a templated histogram.

EDIT (@YannickJadoul):
Closes#1692

YannickJadoul

Damn, @henryiii, you beat me to it, again ;-)

+1 for adding this!

When considering this PR over the last year, I realized this only works for class_-registered types. So, given that we'll undoubtedly get lots of questions/issues about "why doesn't py::type<int>() not return the int type object?":

Should we extend this? Yes, your documentation says it's for exposed types, so it's not strictly necessary. But I do wonder how many "bug" reports we're going to get. Plus, it would be a nice way of getting hold of the int type object?
Is there an easy way to do so? One possibility I can see would be to add this to type_caster as static py::object type() (or static py::handle type()) and add the current implementation to type_caster_base? Not sure if this is a great idea, but it would a) be extensible, and b) be backwards compatible (since you'll get a compile error if the type_caster doesn't have a static type function).

EricCousineau-TRI · 2020-08-09T17:25:21Z

Thanks!!! I've had some code that, uh, dipped into the internals for a while now 😅
https://github.com/RobotLocomotion/drake/blob/26bf5dca63fe0c63df940ef30c2e3b5196c4339a/bindings/pydrake/common/cpp_param_pybind.cc#L101-L102

@YannickJadoul Unfortunately, certain users may have certain desires for py::type(). I, for one, would like to have the option to direct it towards the closest NumPy / ctype, and I register my own aliases to ensure that I can distinguish types like uint8_t, int64_t, etc., whereas using the type_caster approach may scrub out those details.

EDIT: Thinking on it more, given that I already intercept typeids first to handle my aliases, I guess I wouldn't be hurt by it...

I am totes fine with this only dealing with registered types. Perhaps, then renaming it to something slightly more obtuse, like py::generic_type, or something like that to align with the hierarchy of casters using type_caster_generic?
Perhaps terminology from the docs could help steer this?
https://pybind11.readthedocs.io/en/stable/advanced/cast/index.html#type-conversions

YannickJadoul · 2020-08-10T13:17:43Z

I am totes fine with this only dealing with registered types. Perhaps, then renaming it to something slightly more obtuse, like py::generic_type, or something like that to align with the hierarchy of casters using type_caster_generic?

I like the py::type name, though, so I'd prefer to keep it. But how hard would it be to make using another type for this fail compiling? Could we have some enable_if or static_assert checking std::is_base_of<py::detail::generic_type_caster<the_same_kind_of_decay_as_in_make_caster<...>>, py::detail::make_caster<...>>::value?

EricCousineau-TRI · 2020-08-10T13:43:29Z

Yeah, I think a fail fast is achievable. Perhaps with something like this in the function body?

static_assert(
is_generic_type_v<T>,
"This only works for registered C++ types. The type here is most likely type converted (using type_caster).")

YannickJadoul · 2020-08-10T13:55:34Z

is_generic_type_v<T>

Except that we don't have that, I believe? So it'll be something like checking that a subclass of the default type_caster_base or type_caster_generic is used? (Actually, I think it's type_caster_base that does this mapping between T and the Python type, no?)

henryiii · 2020-08-10T14:47:06Z

If we have it fail for now with non-user registered types, it could always be added later - changing a failure to a success is always allowed. :)

YannickJadoul · 2020-08-10T15:20:54Z

If we have it fail for now with non-user registered types, it could always be added later - changing a failure to a success is always allowed. :)

Exactly! As long as we do it safely (at compile time), this would be perfect!

wjakob · 2020-08-17T08:30:14Z

This looks great, but would you mind adding an explicit discussion in the documentation (the auto-extracted docstrings IIRC do end up in the documentation as well, but rather hidden..)

henryiii · 2020-08-17T19:23:02Z

Waiting pending discussion on #2388

henryiii · 2020-09-09T03:50:15Z

By the way, I'm open to ideas on how to get something like py::type::of<int>() to work. :) Also, should we use py::type::of(h) for the from handle form? And should this be a handle subclass or object?

EricCousineau-TRI · 2020-09-14T14:27:13Z

Filed #2486 ;)

YannickJadoul · 2020-09-14T14:28:28Z

The simplest case would be std::variant - there will be run-time branching of the type, and what they (users) may want may heavily depend on their use case.

Right, but then for std::variant, you can never do that? py::type::of<T> doesn't get an instance, anyway?

Should this return something like Typing.Variant[...], where ... is filled in with the inner type-of expressions, only for Python 3.6+?

That's an interesting suggestion, though, but I'm not sure. It's not really a type, right? Can you create an object using this?

And that's a relatively easy case. What about when users may actually want to distinguish between int32 and uint32?

But these do get mapped to the same Python type? I don't see the problem, here? There's a mismatch between C++ and Python, but you can still say "If you pass a C++ type T, you'll get a Python type py::type::of<T>. At least it's better than nothing.
And it provides a way to get the int type object, through py::type::of<int>()? (Maybe there's also a better API, indeed.)

I mean, having py::type::of<int>() could help, I just dunno if that should be lumped into this PR without more discussion, as I think it's more nuanced.

Agreed! Let's get the easy stuff in, just making sure it doesn't stop us from achieving the more complicated stuff :-)

EricCousineau-TRI

Looks awesome!

henryiii · 2020-09-14T14:54:49Z

Right, but then for std::variant, you can never do that? py::type::of doesn't get an instance, anyway?

Without verifying it could be done, I think you could have py::type::of<variant<...>>(instance) working in that case, but only if all the inner types were supported (note that there is an argument above). But I don't think supporting <int> means you have to support <variant<...>>?

Let's get the easy stuff in

Unless it was extremely easy, was not planning to add to this PR :)

YannickJadoul · 2020-09-14T15:10:49Z

+ PYBIND11_OBJECT_COMMON(type, object, PyType_Check)
+
+ explicit type(handle h): type((PyObject*) Py_TYPE(h.ptr()), borrowed_t{}) {}
+ explicit type(object ob): type((PyObject*) Py_TYPE(ob.ptr()), borrowed_t{}) {}


This isn't necessary, I think? object should be a subclass of handle, and there's no extra refcounting going on?

Oh, yes, some slicing, since it's not by reference. But there are precedents for that, I think?

If you/we choose to keep the object overload, better to make it a const object &, then. It avoids an unnecessary refcount.

Ah, no, PYBIND11_OBJECT already adds that overload.
pybind11/include/pybind11/pytypes.h
Lines 813 to 817 in cc982ac
#definePYBIND11_OBJECT(Name, Parent, CheckFun) \
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
/* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
Name(const object &o) : Parent(o) { } \
Name(object &&o) : Parent(std::move(o)) { }
How do we fix this such that it works in the intended way? When do we want py::type(...) to "cast" a py::object to a subclass, and when do we want it to actually call type(...)? Does this mean that @henryiii's original py::type::of is necessary, and that we can't have the py::type(obj) interface I've been dreaming of? :-(

We use PYBIND11_OBJECT_COMMON instead of PYBIND11_OBJECT.

(I'll try dropping this overload soon)

Can this be a const handle &?

We use PYBIND11_OBJECT_COMMON instead of PYBIND11_OBJECT.
🤦 How did I miss that?
Can this be a const handle &?
Probably, but I don't think it's necessary. handle really is a fancy and more beautiful way of writing PyObject *.

YannickJadoul · 2020-09-14T15:46:00Z

Sorry to open that discussion again (after first telling @henryiii to change it), but, I'm now realizing the following:

If py::type(obj) always gets the type of an object; we have no way of actually casting a py::object (being returned from a Python call or so) to be converted? Is that OK, or did I speak too soon, when saying py::type(handle) would be a great API?

henryiii · 2020-09-14T15:54:50Z

I think I preferred py::type::of(h), even though it's a little further from the Python API, both from symmetry with py::type::of<T>() and because it keeps you from casting a known type into a type. (I had tests for this, the behavior of py::type::of was not the same as py::type if you gave it a valid class).

henryiii · 2020-09-14T16:08:32Z

It looks like PYBIND11_OBJECT doesn't throw if the constructor doesn't pass the check function. So only manually checking the ->check is the only way to ensure the object really is a type if you do py::type(object)? I take it there's a reason constructers should not throw in this case? I can manually add it instead of using PYBIND11_OBJECT if there's not a reason.

YannickJadoul · 2020-09-14T16:23:28Z

It looks like PYBIND11_OBJECT doesn't throw if the constructor doesn't pass the check function. So only manually checking the ->check is the only way to ensure the object really is a type if you do py::type(object)? I take it there's a reason constructers should not throw in this case? I can manually add it instead of using PYBIND11_OBJECT if there's not a reason.

I worked on that here in #2349, but it got stalled behind the __qualname__ unification with Python 2 & PyPy.

henryiii · 2020-09-14T16:24:47Z

Good, then leave as is, then we can take advantage of that if it gets merged?

YannickJadoul · 2020-09-14T16:24:50Z

I think I preferred py::type::of(h), even though it's a little further from the Python API, both from symmetry with py::type::of<T>() and because it keeps you from casting a known type into a type. (I had tests for this, the behavior of py::type::of was not the same as py::type if you gave it a valid class).

I really liked py::type(...), but I'm now realizing that will be ambiguous and confusing :-( So yes, sorry for having you make that change to then revert it! py::type::of will be better.

YannickJadoul · 2020-09-14T16:25:10Z

Good, then leave as is, then we can take advantage of that if it gets merged?

Good for me. I'll try to pick that up again, soon'ish...

wjakob · 2020-09-15T08:28:56Z

Thanks everyone!

Pybind11 2.6 introduces `pybind11::type::of<T>()` and `pybind11::type`, see pybind/pybind11#2364. We can use neither: `of<T>()` throws and `type` cannot represent `None`.

bstaletic approved these changes Aug 4, 2020
View reviewed changes

YannickJadoul approved these changes Aug 4, 2020
View reviewed changes

Comment threadinclude/pybind11/cast.h Outdated

EricCousineau-TRI reviewed Aug 9, 2020
View reviewed changes

Comment threadinclude/pybind11/cast.h Outdated

EricCousineau-TRI reviewed Aug 9, 2020
View reviewed changes

Comment threadtests/test_class.cpp

henryiii force-pushed the feat/type branch 2 times, most recently from 681c849 to fd8f6fcCompare August 10, 2020 15:29

YannickJadoul reviewed Aug 10, 2020
View reviewed changes

Comment threadtests/test_class.py Outdated

henryiii force-pushed the feat/type branch from fd8f6fc to 722b527Compare August 10, 2020 15:42

henryiii mentioned this pull request Aug 12, 2020
refactor: Replace h.get_type() with py::type(h)#2388
Closed

henryiii marked this pull request as draft August 17, 2020 19:23

henryiii added the on hold label Aug 18, 2020

henryiii force-pushed the feat/type branch 2 times, most recently from 2174918 to 8a9238dCompare September 8, 2020 15:57

henryiii marked this pull request as ready for review September 8, 2020 16:09

henryiii force-pushed the feat/type branch from 8a9238d to 598ebcdCompare September 8, 2020 16:09

henryiii added enhancement and removed on hold labels Sep 8, 2020

henryiii changed the title ~~feat: type<T>()~~feat: type::of<T>() and type(h)Sep 8, 2020

jagerman reviewed Sep 9, 2020
View reviewed changes

Comment threaddocs/advanced/classes.rst Outdated

henryiii requested review from YannickJadoul and bstaletic September 9, 2020 03:41

bstaletic approved these changes Sep 14, 2020
View reviewed changes

EricCousineau-TRI mentioned this pull request Sep 14, 2020
[FEAT] Should teach py::type::of<> to handle non-generic types #2486
Open

EricCousineau-TRI approved these changes Sep 14, 2020
View reviewed changes

Comment threaddocs/advanced/classes.rst
Comment threadtests/test_class.cpp
Comment threadinclude/pybind11/pytypes.h Outdated
Comment threadtests/test_class.py

docs: some added notes from@EricCousineau-TRI
e950722

EricCousineau-TRI approved these changes Sep 14, 2020
View reviewed changes

YannickJadoul approved these changes Sep 14, 2020
View reviewed changes

henryiii changed the title ~~feat: type::of<T>() and type(h)~~feat: py::type::of<T>() and py::type::of(h)Sep 14, 2020

refactor: use py::type::of
636d5cf

henryiii force-pushed the feat/type branch from ac1a428 to 636d5cfCompare September 14, 2020 17:17

EricCousineau-TRI approved these changes Sep 14, 2020
View reviewed changes

Comment threadinclude/pybind11/cast.h

YannickJadoul approved these changes Sep 14, 2020
View reviewed changes

henryiii merged commit f12ec00 into pybind:masterSep 14, 2020

henryiii deleted the feat/type branch September 14, 2020 22:06

henryiii mentioned this pull request Sep 17, 2020
refactor: replace .get_type with type::handle_of #2492
Merged

YannickJadoul mentioned this pull request Nov 27, 2020
[BUG] py::type constructor from object does not throw when passed a non-type object #2700
Closed

Skylion007 mentioned this pull request Aug 19, 2021
esp::core::Configuration changed to use unordered_map of variant-like instead of Magnum::ConfigurationGroups facebookresearch/habitat-sim#1433
Merged
11 tasks

rwgk mentioned this pull request Feb 10, 2023
FWD pybind11 google/pybind11clif#2364
Closed

	#definePYBIND11_OBJECT(Name, Parent, CheckFun) \
	PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
	/* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
	Name(const object &o) : Parent(o) { } \
	Name(object &&o) : Parent(std::move(o)) { }

Uh oh!

Conversation

henryiii commented Aug 4, 2020• edited by YannickJadoul Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YannickJadoul left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

EricCousineau-TRI commented Aug 9, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

YannickJadoul commented Aug 10, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricCousineau-TRI commented Aug 10, 2020

Uh oh!

YannickJadoul commented Aug 10, 2020

Uh oh!

henryiii commented Aug 10, 2020

Uh oh!

YannickJadoul commented Aug 10, 2020

Uh oh!

Uh oh!

wjakob commented Aug 17, 2020

Uh oh!

henryiii commented Aug 17, 2020

Uh oh!

Uh oh!

henryiii commented Sep 9, 2020

Uh oh!

EricCousineau-TRI commented Sep 14, 2020

Uh oh!

YannickJadoul commented Sep 14, 2020

Uh oh!

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

henryiii commented Sep 14, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YannickJadoul commented Sep 14, 2020

Uh oh!

henryiii commented Sep 14, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henryiii commented Sep 14, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YannickJadoul commented Sep 14, 2020

Uh oh!

henryiii commented Sep 14, 2020

Uh oh!

YannickJadoul commented Sep 14, 2020

Uh oh!

YannickJadoul commented Sep 14, 2020

Uh oh!

henryiii commented Aug 4, 2020•
edited by YannickJadoul
Loading

YannickJadoul left a comment •
edited
Loading

EricCousineau-TRI commented Aug 9, 2020•
edited
Loading

YannickJadoul commented Aug 10, 2020•
edited
Loading

henryiii commented Sep 14, 2020•
edited
Loading

henryiii commented Sep 14, 2020•
edited
Loading

henryiii commented Sep 14, 2020•
edited
Loading