Add support for non-converting arguments

[jagerman]

This adds support for controlling the convert flag of arguments through the py::arg annotation. This then allows arguments to be flagged as non-converting, which the type_caster<X> is able to use to implement different behaviour.

Currently, as far as I can see, convert is only used for type converters of regular pybind11-registered types (to prevent implicit conversions from recursing); all of the other core type_casters just ignore the argument. We can, however, repurpose it to control internal conversion of converters like Eigen and array: most usefully to give callers a way to disable the conversion that would otherwise occur when a Eigen::Ref<const Eigen::Matrix> argument is passed a numpy array that requires conversion (either because it has an incompatible stride or the wrong dtype).

Specifying a noconvert argument looks like one of these:

    m.def("f1", &f, "a"_a.noconvert() = "default"); // Named, default, noconvert
    m.def("f2", &f, "a"_a.noconvert()); // Named, no default, no converting
    m.def("f3", &f, py::arg().noconvert()); // Unnamed, no default, no converting

(The last one—being able to declare a py::arg without a name—is new: previously py::arg() only accepted named keyword arguments).

Such an non-convert argument is then passed convert = false by the type caster when loading the argument. Whether this has an effect is up to the type caster itself, but as mentioned above, this would be extremely helpful for the Eigen support to give a nicer way to specify a "no-copy" mode than the custom wrapper in the current PR, and moreover isn't an Eigen-specific hack.

The relevant bit of the internal Eigen type caster would then contain something like:

bool load(handle src, bool convert) {
    // ...
    bool copy_required = (stride_incompatible || wrong_dtype);
    if (copy_required && !convert) return false;
    // ...
}

and binding code that wanted to say "do not copy data" would then use:

void f(Eigen::Ref<const MatrixXd> m) { /* ... */ }

    // Binding code:
    m.def("f", &f, py::arg().noconvert());

rather than the current PR's ugly proposal of:

    m.def("f", [](py::EigenRefNoCopy<const MatrixXd> m) { return f(m); });

While that workaround works for Eigen, it's not exactly elegant, and I think this no-convert argument could be useful elsewhere, too.

Thoughts/comments/suggestions?

[aldanor]

I had the same thoughts recently, see #338 (comment)

Basically, we need to be able to accept "lvalues" (e.g. NumPy arrays that are to mutated in place), which is something that's not possible with auto conversion.

Will read this in more detail and reply tomorrow :)

[jagerman]

Yeah, your #338 comments were part of the reason to do this globally rather than just the Eigen hack.

[wjakob]

I didn't understand this change -- why the rvalue?

[jagerman]

It was to potentially avoid a copy--that's now just a private constructor with the common code, invoked with an rvalue arg from the direct (first public) or converted-from-arg (second public) constructors.

[wjakob]

I think this is a great API -- it's simple to use and leads to nice and readable binding code. Questions from my end would be:

1. Is 1 bit enough for what we'll need to do in the future?
2. You guessed it: does this increase .SO size? ;)

[jagerman]

Is 1 bit enough for what we'll need to do in the future?

That depends a lot on the future, I guess. I toyed with a change to pass a custom argument info struct into load (with some clever detection of whether a type_caster has a load(str, convert) or a a load(str, argument_info) method). Unfortunately it required some pretty major changes to type caster invocation, not contained just to the argument loader. One advantage of this approach is that it doesn't break existing type casters.

You guessed it: does this increase .SO size? ;)

I measured this, and meant to include it in the PR message (but apparently forgot). Commenting out the added tests, the .so increase (under gcc) is pretty small:

------ pybind11_tests.cpython-35m-x86_64-linux-gnu.so file size: 1204192 (change of +4224 bytes = +0.35%)

[jagerman]

One other thing that I had in mind when writing this PR (but that belongs in a future PR) is that it makes it reasonably easy to implement a two-pass function call procedure, where we first try to call the function with convert = false on all the arguments, then, if that fails, go through the chain again with convert = true (except for those with py::arg().noconvert()). That (combined with py::array_t changes to understand the convert flag) would address the issue in #631: if you bind several overloads which all work via conversion, but one works without conversion, we would still invoke the one that doesn't require conversion (then fallback to the first if none work without conversion).

[wjakob]

Merged!