-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[BUG] Clearer commitment to public / stable API? (most importantly, type_caster) #2763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
The docs acknowledge this, though, and explicitly say it is allowed: https://pybind11.readthedocs.io/en/stable/advanced/cast/custom.html I would first focus on fixing a few things, then use what wa learned (e.g., what things would be nice to allow to be customized from the outside) to fuel a redesign? There's already a |
Quoting from there: "In very rare cases, ..." This premise seems far removed from reality (to avoid the word "wrong"). Interesting experiment to get scientific numbers (and possibly also a way to clean house): rename |
Oh, I definitely agree, there. And as far as I'm concerned, custom
I'm not convinced about that, though. These should be for rare, specialized cases where you need full control. Often, you can (should?) just wrap things with a lambda. My main point in linking to that part of the docs that is that
To be fair, apart from casters (which are explicitly allowed, caveats about semi-unstable API included), nothing should depend on Moreover, ríght now, as we're planning to fix a bunch of holder and caster stuff after bumping into the limits of the current design, would be (IMO) the very worst time to commit to a fully public API. Summary: this is not me dismissing that this would be very nice to have on the longer term; it would definitely be and be an important step towards a maturing pybind11! But committing to the current API would be a grave mistake, I believe. |
I think I'll do the suggested experiment, to get actual numbers and see where we really stand. Maybe I'm missing something (a sincere question)? |
That could definitely be interesting!
Apart from |
At present,
type_caster
is something that exists in thepy::detail
namespace, but may be overloaded in downstream applications, either via a macro, or more directly.A naive downstream example (that I wrote 😬):
https://github.com/RobotLocomotion/drake/blob/v0.25.0/bindings/pydrake/common/wrap_pybind.h#L54-L114
https://github.com/RobotLocomotion/drake/blob/v0.25.0/bindings/pydrake/common/cpp_param_pybind.h#L149-L158
At present, my reading of
py::detail
is that it's generally internal implementation details, i.e. it's internal, not public API, and should not be expected to be stable.It seems like the base contract of
py::detail::type_caster
may more-or-less be leaked into a public details. We may want to consider making this more public, and having some level of public stability for this. In this way, we could hoist it outside ofpy::detail
, so we could keep that reserved for truly internal details.Possibly related:
\cc @rwgk @YannickJadoul @rhaschke @wjakob
The text was updated successfully, but these errors were encountered: