Pass a reference to C++ container to the Python callback

[uentity]

Hi!

I faced this issue while implementing a Python's os.walk-like function for my tree structure. I hoped to mimic the ability of callback to edit passed list of directories in-place and guide further search.

Suppose we have the following in C++:

#include <pybind11/stl_bind.h>

PYBIND11_MAKE_OPAQUE(std::vector<double>);
namespace py = pybind11;
...

py::bind_vector<std::vector<double>>(m, "d_list", py::module_local(false));
// this will work as expected because I can explicitly point that return value is a reference
m.def("test_static_d", []() -> std::vector<double>& {
	static std::vector<double> v{0, 0};
	return v;
}, py::return_value_policy::reference);

// callback will fail to edit passed vector, because a copy is involved somewhere?
m.def("test_callback", [](std::function<void(std::vector<double>&)> f) {
	std::vector<double> v{0, 0};
	f(v);
	for(const auto& l : v) {
		std::cout << l << ' ';
	}
	std::cout << std::endl;
});

And now the Python code:

In [2]: a = example.test_static_d()

In [3]: a.append(1)

In [4]: a
Out[4]: d_list[0, 0, 1]

In [5]: b = example.test_static_d()

In [6]: b
Out[6]: d_list[0, 0, 1]
# so far so good
In [7]: b is a
Out[7]: True

# callback seems to modify a copy of vector
def f(v) :
 v.append(1.)
 print(v)

# C++ function don't get updated vector after callback invoke
In [1]: example.test_callback(f)
d_list[0, 0, 1]
0 0 

Is there a way to specify that callback should take an opaque container by reference (in order to modify it in-place on Python side)?

[EricCousineau-TRI]

Possibly related to #1231: This looks like it might also be a symptom of argument reference policies - it is most like likely passing a copy of your vector to your function.

Can you try making your signature into std::function<void (std::vector<double>*)>?

[jagerman]

The main issue here in how return-value-policies propagate (or rather, don't propagate) through std::function bindings. You can see this in functional.h: the value = [func](...) lambda simply calls the py::function argument with its arguments, which ends up doing ordinary argument casting or arguments--which defaults to copying. Realistic, what we want in a case like this is to be able to call (cutting away error handling etc. for exposition):

py::function f = reinterpret_borrow<py::function>(src);
py::object retval = f(py::cast(std::forward<Args>(args), py::return_value_policy::reference)...);
return retval.template cast<Return>();

but of course, that isn't right in many other cases: what we're missing is a way to specify the py::return_value_policy::* values for the inner function arguments.

[uentity]

Jason @jagerman, totally agree with an approach of having argument_policy the same way as we already have return_value_policy. Is it ever planned to be implemented?

[EricCousineau-TRI]

My gut feeling is that explicitly specifying return value policies make sense for return values, but explicitly specifying input argument policies seems like it'd introduce an extra layer of complication (versus simply treating mutable lvalue references as reference-only, no copy).

Can I ask what workflows there are where a (temporary) copy of an object for a mutable lvalue reference is actually desirable?

That being said, it may line up more nicely with ownership transfer, like @torokati44's PR #1226.
Though for that, if this were to be used for ownership), what would the policy be for take_ownership (implying a contextual definition)? Or would it be a different flag, that's not really a return-value policy per-se, but a casting policy?
And what would the interplay look like for holder types? (Transferring ownership to C++ for a copyable holder doesn't make any sense, and you can't release() a shared_ptr if the input argument is a pointer.)

(I do have a little bit of bias here, as it'd be nice to let referencing / ownership be implicit for arguments, as they are done in C++, through the use of mutable lvalue references, unique_ptr, etc :) )

[varunagrawal]

@jagerman @uentity any updates on this issue? We're facing a similar issue and would really like a resolution or to help with a way forward.