pybind11
pybind11 copied to clipboard
pybind
add argument number dispatch mechanism for std::function casting
Description
The proposed PR partly fixing function overloads where the arguments are std::functions.
It fixes #3035.
The problem appears, if we have at least two functions:
PYBIND11_EMBEDDED_MODULE(func_module, m) {
m.def("func", [](const std::function<int(int, int)>& f) { return f(2, 3); })
.def("func", [](const std::function<int(int)>& f) { return f(2); });
}
and we want to call them in Python
import func_module
def f(a):
return a*2
func_module.func(f)
The function f is cast to the first overload, which takes two arguments.
Then the call fails with
f() takes 1 positional argument but 2 were given
Calling it from c++ with
auto f = std::function([](int x) { return 2 * x; });
auto m = py::module_::import("func_module");
m.attr("func")(f);
fails with
)m.attr(" func ")(f).cast<int>() failed with: \nTypeError: (): incompatible function arguments. The following argument types are supported:
1. (arg0: int) -> int
Invoked with: 2, 3
This is fixed with this PR.
In Compiler Explorer I created the failing example, see if you want https://godbolt.org/z/c4Y9bTxTE
Suggested changelog entry:
Number of arguments are now checked for casting `std::function`.
I'm wondering about the tradeoff: extra-code-complexity vs. usability-benefit
For example:
m.def("func", [](const std::function<int(int, int)>& f) { return f(2, 3); })
.def("func", [](const std::function<int(int)>& f) { return f(2); });
This could simply become something like:
m.def("handle_point", [](const std::function<int(int, int)>& f) { return f(2, 3); })
.def("handle_scalar", [](const std::function<int(int)>& f) { return f(2); });
Meaningful names will make the client code more obvious and readable.
When is it important that the names are identical?
I think you are right and this is one workaround. We used it in our code but we changed the code to just accept a py::object and inspect it inside C++ using the inspect module. I didn't like this boilerplate everywhere in my library.
I have more complicated types as arguments, where it does not make much sense to change the function name, e.g.
in my use case I have a C++ API, which I cannot change and it would be nice to have the same function names for the Python bindings to not confuse users.
For C++, IMHO this kind of function overloading is so common, the fact that it is not supported, confuses Python bindings writers, which come from the C++ world.
For me this use case is not different to just having a Python tuple converted to an std::tuple, where this dispatch works, doesn't it?
To stay in the example I don't want to write
m.def("handle_point2D", [](const std::function<int(int, int)>& f) { return f(2, 3); })
m.def("handle_point3D", [](const std::function<int(int, int, int)>& f) { return f(2, 3, 4); })
but I just want to have a single function handle_point, that works for 2D and 3D.
This dispatch could also be done within Python by providing a wrapper fun tion but I think it is clear what I mean.
So I think the dispatch should be added to really dispatch this case or at least to show an exception that really states the problem, since for me it also took a while to figure out, why it always tries to call the wrong overload.
If this is not added it would be nice to explicitly state it in the documentation, that this is not supported.
but I just want to have a single function
handle_point, that works for 2D and 3D.
Understood, and if the implementation was easier, I'd say fine, why not.
I'm worried about relying on __code__ and __call__ attribute lookups (runtime overhead; also might be fragile in the future).
I haven't looked in great detail, but I'm pretty sure the error handling is incomplete. This will add to the existing code.
So it comes down to judgement, cost vs benefit. I think it'll be "too much code" for "a relative niche feature".
But I'm not the only one here: @henryiii @EthanSteinberg @Skylion007 What's your opinion?
I think this argument number dispatch is a good feature that will make it easier to write pythonic bindings (as python code generally leans in the direction of functions doing type dispatching).
I am a bit worried about runtime overhead though. At minimum, we probably only want to do this check if there is more than one overload.
Let me give this a proper review
Ok actually I'm wittnessing some problems in different CI configurations. I will let you know when this is ready again
What happens if a function with default arguments is passed in, like f(x, y=1)? Also, is f(x, *, y=1) consider 1 arg (it should be)?
