feat: py::pos_only

[henryiii]

This adds a positional-only marker to pybind11, similar to Python 3.8.

It also provides the correct signature for py::kwonly() along with py::pos_only(), and the signatures for these two options are now tested.

Example:

m.def("pos_kw_only_mix", [](int i, int j, int k) { return py::make_tuple(i, j, k); },
      py::arg("i"), py::pos_only(), py::arg("j"), py::kwonly(), py::arg("k"));

Produces the following signature:

pos_kw_only_mix(i: int, /, j: int, *, k: int) -> tuple

The final commit renames kwonly to kw_only since that seems to be winning in #2410.

Closes #2410

[henryiii]

FYI, the AppVeyor build is failing because of Eigen:

Repository unavailable
Bitbucket no longer supports Mercurial repositories.

[YannickJadoul]

Looks sleek and good to me, on first quick review :-) Great work, @henryiii!

[YannickJadoul]

Does this need to be set? It does seem like we could just by default set r->nargs_pos_only to 0 and increase it when encountered?

[YannickJadoul]

Note: Python doesn't seem to allow / before any arguments:

>>> def f(/, a, b, *, c=42): pass
  File "<stdin>", line 1
    def f(/, a, b, *, c=42): pass
          ^
SyntaxError: invalid syntax

So we don't need to worry about producing that signature when has_pos_only_args is true, but nargs_pos_only is false.
If anything, we might even throw some exception if r->args.size() is 0? Or even better, catch it in the static_assert in cpp_function. But not strictly necessary for me, either. We can just allow and ignore it.

[henryiii]

It was avoidable, yes! Removed. Not sure how to test this properly at compile time (adding a runtime check would be quite easy).

[YannickJadoul]

Rather than any_of<std::is_same<pos_only, Extra>...>::value, we could write write our own first_of, and compare if the index of pos_only is before the first arg or arg_v. I can try something out, but I agree it might not be worth it, indeed.

Besides that, we might want to add a check that there's only 1 kw_only and only 1 pos_only, maximally? There will be weird effects, otherwise, no?

[henryiii]

There are several possible ways to improve argument handling, but that probably doesn't have to be in this PR. Ideas:

• (a, *args, b=None) is valid in Python but not pybind11.
• (a, /, **kwargs) allows "a=" to go into kwargs
• (a, / a) isn't valid in Python, but we could allow it
• We could check to make sure py::pos_only() is not at the beginning and py::kw_only() is not at the end.
• We could have a nice error if either was given twice or in the wrong order.

[YannickJadoul]

OK, then there are more problems than I thought, yes.

(a, /, **kwargs) allows "a=" to go into kwargs

This works in Python 3.8 as well, though:

>>> def f(a, /, **kwargs): pass
... 
>>> f(21)
>>> f(21, a=42)
>>> 

[henryiii]

Everything i listed is valid in Python 3.8 except (a, /, a) (after all, what would a be?). We could allow it, though, since the name is not the same as the C++ argument for us.

[YannickJadoul]

Oh, then I misunderstood.

We could allow it, though, since the name is not the same as the C++ argument for us.

True, but it would be really bad design, and should always easily fixable, I'd say. So sticking with Python-conventions would make things easier, I'd say.

[YannickJadoul]

FYI, the AppVeyor build is failing because of Eigen:

Repository unavailable
Bitbucket no longer supports Mercurial repositories.

Ugh :-|

[YannickJadoul]

Should we try merging 46d56f6 separately? Other PRs are going to fall over this as well.

[henryiii]

Should we try merging

Azure is passing, so I'll cherry-pick it to master.

[bstaletic]

No major questions/complaints, because @YannickJadoul has done a great job at that already.

[bstaletic]

I haven't checked the rest of the codebase, so if this is consistent, leave it. I'm just a little bothered by else block not having braces while the if block does.

[henryiii]

It's done the same way just below, in the keyword checking.

[bstaletic]

Looks good from here.

[YannickJadoul]

OK, if we decide that extra error handling is for another PR (fine with me), then this looks excellent to me! :-)

[wjakob]

This looks amazing @henryiii ! No change requests from me -- thank you also for the feedback @YannickJadoul and @bstaletic!