Zengtudor/pybind11
1
0
Fork 0
mirror of https://github.com/pybind/pybind11.git synced 2024-11-25 06:35:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: allow kw-only args after a py::args (#3402)

Browse Source 
* Simply has_kw_only_args handling

This simplifies tracking the number of kw-only args by instead tracking
the number of positional arguments (which is really what we care about
everywhere this is used).

* Allow keyword-only arguments to follow py::args

This removes the constraint that py::args has to be last (or
second-last, with py::kwargs) and instead makes py::args imply
py::kw_only for any remaining arguments, allowing you to bind a function
that works the same way as a Python function such as:

    def f(a, *args, b):
        return a * b + sum(args)

    f(10, 1, 2, 3, b=20)  # == 206

With this change, you can bind such a function using:

    m.def("f", [](int a, py::args args, int b) { /* ... */ },
        "a"_a, "b"_a);

Or, to be more explicit about the keyword-only arguments:

    m.def("g", [](int a, py::args args, int b) { /* ... */ },
        "a"_a, py::kw_only{}, "b"_a);

(The only difference between the two is that the latter will fail at
binding time if the `kw_only{}` doesn't match the `py::args` position).

This doesn't affect backwards compatibility at all because, currently,
you can't have a py::args anywhere except the end/2nd-last.

* Take args/kwargs by const lvalue ref

Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>

Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
This commit is contained in:
Jason Rhinelander 2021-10-29 00:16:55 -03:00 committed by GitHub
parent a80b22374a
commit e7c9753f1d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 186 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
docs/advanced/functions.rst
View File

@ -306,8 +306,9 @@ The class ``py::args`` derives from ``py::tuple`` and ``py::kwargs`` derives
from ``py::dict``.
You may also use just one or the other, and may combine these with other
arguments as long as the ``py::args`` and ``py::kwargs`` arguments are the last
arguments accepted by the function.
arguments. Note, however, that ``py::kwargs`` must always be the last argument
of the function, and ``py::args`` implies that any further arguments are
keyword-only (see :ref:`keyword_only_arguments`).
Please refer to the other examples for details on how to iterate over these,
and on how to cast their entries into C++ objects. A demonstration is also
@ -366,6 +367,8 @@ like so:
py::class_<MyClass>("MyClass")
.def("myFunction", py::arg("arg") = static_cast<SomeType *>(nullptr));
.. _keyword_only_arguments:
Keyword-only arguments
======================
@ -397,6 +400,15 @@ feature does *not* require Python 3 to work.
.. versionadded:: 2.6
As of pybind11 2.9, a ``py::args`` argument implies that any following arguments
are keyword-only, as if ``py::kw_only()`` had been specified in the same
relative location of the argument list as the ``py::args`` argument. The
``py::kw_only()`` may be included to be explicit about this, but is not
required. (Prior to 2.9 ``py::args`` may only occur at the end of the argument
list, or immediately before a ``py::kwargs`` argument at the end).
.. versionadded:: 2.9
Positional-only arguments
=========================

28
include/pybind11/attr.h
View File

@ -174,7 +174,7 @@ struct function_record {
function_record()
: is_constructor(false), is_new_style_constructor(false), is_stateless(false),
is_operator(false), is_method(false), has_args(false),
has_kwargs(false), has_kw_only_args(false), prepend(false) { }
has_kwargs(false), prepend(false) { }
/// Function name
char *name = nullptr; /* why no C++ strings? They generate heavier code.. */
@ -221,17 +221,15 @@ struct function_record {
/// True if the function has a '**kwargs' argument
bool has_kwargs : 1;
/// True once a 'py::kw_only' is encountered (any following args are keyword-only)
bool has_kw_only_args : 1;
/// True if this function is to be inserted at the beginning of the overload resolution chain
bool prepend : 1;
/// Number of arguments (including py::args and/or py::kwargs, if present)
std::uint16_t nargs;
/// Number of trailing arguments (counted in `nargs`) that are keyword-only
std::uint16_t nargs_kw_only = 0;
/// Number of leading positional arguments, which are terminated by a py::args or py::kwargs
/// argument or by a py::kw_only annotation.
std::uint16_t nargs_pos = 0;
/// Number of leading arguments (counted in `nargs`) that are positional-only
std::uint16_t nargs_pos_only = 0;
@ -411,10 +409,9 @@ template <> struct process_attribute<is_new_style_constructor> : process_attribu
static void init(const is_new_style_constructor &, function_record *r) { r->is_new_style_constructor = true; }
};
inline void process_kw_only_arg(const arg &a, function_record *r) {
if (!a.name || a.name[0] == '\0')
pybind11_fail("arg(): cannot specify an unnamed argument after an kw_only() annotation");
++r->nargs_kw_only;
inline void check_kw_only_arg(const arg &a, function_record *r) {
if (r->args.size() > r->nargs_pos && (!a.name || a.name[0] == '\0'))
pybind11_fail("arg(): cannot specify an unnamed argument after a kw_only() annotation or args() argument");
}
/// Process a keyword argument attribute (*without* a default value)
@ -424,7 +421,7 @@ template <> struct process_attribute<arg> : process_attribute_default<arg> {
r->args.emplace_back("self", nullptr, handle(), true /*convert*/, false /*none not allowed*/);
r->args.emplace_back(a.name, nullptr, handle(), !a.flag_noconvert, a.flag_none);
if (r->has_kw_only_args) process_kw_only_arg(a, r);
check_kw_only_arg(a, r);
}
};
@ -457,14 +454,16 @@ template <> struct process_attribute<arg_v> : process_attribute_default<arg_v> {
}
r->args.emplace_back(a.name, a.descr, a.value.inc_ref(), !a.flag_noconvert, a.flag_none);
if (r->has_kw_only_args) process_kw_only_arg(a, r);
check_kw_only_arg(a, r);
}
};
/// Process a keyword-only-arguments-follow pseudo argument
template <> struct process_attribute<kw_only> : process_attribute_default<kw_only> {
static void init(const kw_only &, function_record *r) {
r->has_kw_only_args = true;
if (r->has_args && r->nargs_pos != static_cast<std::uint16_t>(r->args.size()))
pybind11_fail("Mismatched args() and kw_only(): they must occur at the same relative argument location (or omit kw_only() entirely)");
r->nargs_pos = static_cast<std::uint16_t>(r->args.size());
}
};
@ -472,6 +471,9 @@ template <> struct process_attribute<kw_only> : process_attribute_default<kw_onl
template <> struct process_attribute<pos_only> : process_attribute_default<pos_only> {
static void init(const pos_only &, function_record *r) {
r->nargs_pos_only = static_cast<std::uint16_t>(r->args.size());
if (r->nargs_pos_only > r->nargs_pos)
pybind11_fail("pos_only(): cannot follow a py::args() argument");
// It also can't follow a kw_only, but a static_assert in pybind11.h checks that
}
};

20
include/pybind11/cast.h
View File

@ -1130,6 +1130,9 @@ constexpr arg operator"" _a(const char *name, size_t) { return arg(name); }
PYBIND11_NAMESPACE_BEGIN(detail)
template <typename T> using is_kw_only = std::is_same<intrinsic_t<T>, kw_only>;
template <typename T> using is_pos_only = std::is_same<intrinsic_t<T>, pos_only>;
// forward declaration (definition in attr.h)
struct function_record;
@ -1165,17 +1168,18 @@ class argument_loader {
template <typename Arg> using argument_is_args = std::is_same<intrinsic_t<Arg>, args>;
template <typename Arg> using argument_is_kwargs = std::is_same<intrinsic_t<Arg>, kwargs>;
// Get args/kwargs argument positions relative to the end of the argument list:
static constexpr auto args_pos = constexpr_first<argument_is_args, Args...>() - (int) sizeof...(Args),
kwargs_pos = constexpr_first<argument_is_kwargs, Args...>() - (int) sizeof...(Args);
// Get kwargs argument position, or -1 if not present:
static constexpr auto kwargs_pos = constexpr_last<argument_is_kwargs, Args...>();
static constexpr bool args_kwargs_are_last = kwargs_pos >= - 1 && args_pos >= kwargs_pos - 1;
static_assert(args_kwargs_are_last, "py::args/py::kwargs are only permitted as the last argument(s) of a function");
static_assert(kwargs_pos == -1 || kwargs_pos == (int) sizeof...(Args) - 1, "py::kwargs is only permitted as the last argument of a function");
public:
static constexpr bool has_kwargs = kwargs_pos < 0;
static constexpr bool has_args = args_pos < 0;
static constexpr bool has_kwargs = kwargs_pos != -1;
// py::args argument position; -1 if not present.
static constexpr int args_pos = constexpr_last<argument_is_args, Args...>();
static_assert(args_pos == -1 || args_pos == constexpr_first<argument_is_args, Args...>(), "py::args cannot be specified more than once");
static constexpr auto arg_names = concat(type_descr(make_caster<Args>::name)...);

53
include/pybind11/pybind11.h
View File

@ -203,7 +203,7 @@ protected:
conditional_t<std::is_void<Return>::value, void_type, Return>
>;
static_assert(expected_num_args<Extra...>(sizeof...(Args), cast_in::has_args, cast_in::has_kwargs),
static_assert(expected_num_args<Extra...>(sizeof...(Args), cast_in::args_pos >= 0, cast_in::has_kwargs),
"The number of argument annotations does not match the number of function arguments");
/* Dispatch code which converts function arguments and performs the actual function call */
@ -238,17 +238,27 @@ protected:
return result;
};
rec->nargs_pos = cast_in::args_pos >= 0
? static_cast<std::uint16_t>(cast_in::args_pos)
: sizeof...(Args) - cast_in::has_kwargs; // Will get reduced more if we have a kw_only
rec->has_args = cast_in::args_pos >= 0;
rec->has_kwargs = cast_in::has_kwargs;
/* Process any user-provided function attributes */
process_attributes<Extra...>::init(extra..., rec);
{
constexpr bool has_kw_only_args = any_of<std::is_same<kw_only, Extra>...>::value,
has_pos_only_args = any_of<std::is_same<pos_only, Extra>...>::value,
has_args = any_of<std::is_same<args, Args>...>::value,
has_arg_annotations = any_of<is_keyword<Extra>...>::value;
static_assert(has_arg_annotations || !has_kw_only_args, "py::kw_only requires the use of argument annotations");
static_assert(has_arg_annotations || !has_pos_only_args, "py::pos_only requires the use of argument annotations (for docstrings and aligning the annotations to the argument)");
static_assert(!(has_args && has_kw_only_args), "py::kw_only cannot be combined with a py::args argument");
static_assert(constexpr_sum(is_kw_only<Extra>::value...) <= 1, "py::kw_only may be specified only once");
static_assert(constexpr_sum(is_pos_only<Extra>::value...) <= 1, "py::pos_only may be specified only once");
constexpr auto kw_only_pos = constexpr_first<is_kw_only, Extra...>();
constexpr auto pos_only_pos = constexpr_first<is_pos_only, Extra...>();
static_assert(!(has_kw_only_args && has_pos_only_args) || pos_only_pos < kw_only_pos, "py::pos_only must come before py::kw_only");
}
/* Generate a readable signature describing the function's arguments and return value types */
@ -259,9 +269,6 @@ protected:
// Pass on the ownership over the `unique_rec` to `initialize_generic`. `rec` stays valid.
initialize_generic(std::move(unique_rec), signature.text, types.data(), sizeof...(Args));
if (cast_in::has_args) rec->has_args = true;
if (cast_in::has_kwargs) rec->has_kwargs = true;
/* Stash some additional information used by an important optimization in 'functional.h' */
using FunctionType = Return (*)(Args...);
constexpr bool is_function_ptr =
@ -340,16 +347,18 @@ protected:
/* Generate a proper function signature */
std::string signature;
size_t type_index = 0, arg_index = 0;
bool is_starred = false;
for (auto *pc = text; *pc != '\0'; ++pc) {
const auto c = *pc;
if (c == '{') {
// Write arg name for everything except *args and **kwargs.
if (*(pc + 1) == '*')
is_starred = *(pc + 1) == '*';
if (is_starred)
continue;
// Separator for keyword-only arguments, placed before the kw
// arguments start
if (rec->nargs_kw_only > 0 && arg_index + rec->nargs_kw_only == args)
// arguments start (unless we are already putting an *args)
if (!rec->has_args && arg_index == rec->nargs_pos)
signature += "*, ";
if (arg_index < rec->args.size() && rec->args[arg_index].name) {
signature += rec->args[arg_index].name;
@ -361,7 +370,7 @@ protected:
signature += ": ";
} else if (c == '}') {
// Write default value if available.
if (arg_index < rec->args.size() && rec->args[arg_index].descr) {
if (!is_starred && arg_index < rec->args.size() && rec->args[arg_index].descr) {
signature += " = ";
signature += rec->args[arg_index].descr;
}
@ -369,6 +378,7 @@ protected:
// argument, rather than before like *
if (rec->nargs_pos_only > 0 && (arg_index + 1) == rec->nargs_pos_only)
signature += ", /";
if (!is_starred)
arg_index++;
} else if (c == '%') {
const std::type_info *t = types[type_index++];
@ -395,7 +405,7 @@ protected:
}
}
if (arg_index != args || types[type_index] != nullptr)
if (arg_index != args - rec->has_args - rec->has_kwargs || types[type_index] != nullptr)
pybind11_fail("Internal error while parsing type signature (2)");
#if PY_MAJOR_VERSION < 3
@ -631,7 +641,7 @@ protected:
named positional arguments weren't *also* specified via kwarg.
2. If we weren't given enough, try to make up the omitted ones by checking
whether they were provided by a kwarg matching the `py::arg("name")` name. If
so, use it (and remove it from kwargs; if not, see if the function binding
so, use it (and remove it from kwargs); if not, see if the function binding
provided a default that we can use.
3. Ensure that either all keyword arguments were "consumed", or that the function
takes a kwargs argument to accept unconsumed kwargs.
@ -649,7 +659,7 @@ protected:
size_t num_args = func.nargs; // Number of positional arguments that we need
if (func.has_args) --num_args; // (but don't count py::args
if (func.has_kwargs) --num_args; // or py::kwargs)
size_t pos_args = num_args - func.nargs_kw_only;
size_t pos_args = func.nargs_pos;
if (!func.has_args && n_args_in > pos_args)
continue; // Too many positional arguments for this overload
@ -695,6 +705,10 @@ protected:
if (bad_arg)
continue; // Maybe it was meant for another overload (issue #688)
// Keep track of how many position args we copied out in case we need to come back
// to copy the rest into a py::args argument.
size_t positional_args_copied = args_copied;
// We'll need to copy this if we steal some kwargs for defaults
dict kwargs = reinterpret_borrow<dict>(kwargs_in);
@ -747,6 +761,10 @@ protected:
}
if (value) {
// If we're at the py::args index then first insert a stub for it to be replaced later
if (func.has_args && call.args.size() == func.nargs_pos)
call.args.push_back(none());
call.args.push_back(value);
call.args_convert.push_back(arg_rec.convert);
}
@ -769,16 +787,19 @@ protected:
// We didn't copy out any position arguments from the args_in tuple, so we
// can reuse it directly without copying:
extra_args = reinterpret_borrow<tuple>(args_in);
} else if (args_copied >= n_args_in) {
} else if (positional_args_copied >= n_args_in) {
extra_args = tuple(0);
} else {
size_t args_size = n_args_in - args_copied;
size_t args_size = n_args_in - positional_args_copied;
extra_args = tuple(args_size);
for (size_t i = 0; i < args_size; ++i) {
extra_args[i] = PyTuple_GET_ITEM(args_in, args_copied + i);
extra_args[i] = PyTuple_GET_ITEM(args_in, positional_args_copied + i);
}
}
if (call.args.size() <= func.nargs_pos)
call.args.push_back(extra_args);
else
call.args[func.nargs_pos] = extra_args;
call.args_convert.push_back(false);
call.args_ref = std::move(extra_args);
}

17
tests/test_kwargs_and_defaults.cpp
View File

@ -56,6 +56,23 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
m.def("mixed_plus_args_kwargs_defaults", mixed_plus_both,
py::arg("i") = 1, py::arg("j") = 3.14159);
m.def("args_kwonly",
[](int i, double j, const py::args &args, int z) { return py::make_tuple(i, j, args, z); },
"i"_a, "j"_a, "z"_a);
m.def("args_kwonly_kwargs",
[](int i, double j, const py::args &args, int z, const py::kwargs &kwargs) {
return py::make_tuple(i, j, args, z, kwargs); },
"i"_a, "j"_a, py::kw_only{}, "z"_a);
m.def("args_kwonly_kwargs_defaults",
[](int i, double j, const py::args &args, int z, const py::kwargs &kwargs) {
return py::make_tuple(i, j, args, z, kwargs); },
"i"_a = 1, "j"_a = 3.14159, "z"_a = 42);
m.def("args_kwonly_full_monty",
[](int h, int i, double j, const py::args &args, int z, const py::kwargs &kwargs) {
return py::make_tuple(h, i, j, args, z, kwargs); },
py::arg() = 1, py::arg() = 2, py::pos_only{}, "j"_a = 3.14159, "z"_a = 42);
// test_args_refcount
// PyPy needs a garbage collection to get the reference count values to match CPython's behaviour
#ifdef PYPY_VERSION

90
tests/test_kwargs_and_defaults.py
View File

@ -141,6 +141,53 @@ def test_mixed_args_and_kwargs(msg):
""" # noqa: E501 line too long
)
# Arguments after a py::args are automatically keyword-only (pybind 2.9+)
assert m.args_kwonly(2, 2.5, z=22) == (2, 2.5, (), 22)
assert m.args_kwonly(2, 2.5, "a", "b", "c", z=22) == (2, 2.5, ("a", "b", "c"), 22)
assert m.args_kwonly(z=22, i=4, j=16) == (4, 16, (), 22)
with pytest.raises(TypeError) as excinfo:
assert m.args_kwonly(2, 2.5, 22) # missing z= keyword
assert (
msg(excinfo.value)
== """
args_kwonly(): incompatible function arguments. The following argument types are supported:
1. (i: int, j: float, *args, z: int) -> tuple
Invoked with: 2, 2.5, 22
"""
)
assert m.args_kwonly_kwargs(i=1, k=4, j=10, z=-1, y=9) == (
1,
10,
(),
-1,
{"k": 4, "y": 9},
)
assert m.args_kwonly_kwargs(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, z=11, y=12) == (
1,
2,
(3, 4, 5, 6, 7, 8, 9, 10),
11,
{"y": 12},
)
assert (
m.args_kwonly_kwargs.__doc__
== "args_kwonly_kwargs(i: int, j: float, *args, z: int, **kwargs) -> tuple\n"
)
assert (
m.args_kwonly_kwargs_defaults.__doc__
== "args_kwonly_kwargs_defaults(i: int = 1, j: float = 3.14159, *args, z: int = 42, **kwargs) -> tuple\n" # noqa: E501 line too long
)
assert m.args_kwonly_kwargs_defaults() == (1, 3.14159, (), 42, {})
assert m.args_kwonly_kwargs_defaults(2) == (2, 3.14159, (), 42, {})
assert m.args_kwonly_kwargs_defaults(z=-99) == (1, 3.14159, (), -99, {})
assert m.args_kwonly_kwargs_defaults(5, 6, 7, 8) == (5, 6, (7, 8), 42, {})
assert m.args_kwonly_kwargs_defaults(5, 6, 7, m=8) == (5, 6, (7,), 42, {"m": 8})
assert m.args_kwonly_kwargs_defaults(5, 6, 7, m=8, z=9) == (5, 6, (7,), 9, {"m": 8})
def test_keyword_only_args(msg):
assert m.kw_only_all(i=1, j=2) == (1, 2)
@ -178,7 +225,7 @@ def test_keyword_only_args(msg):
assert (
msg(excinfo.value)
== """
arg(): cannot specify an unnamed argument after an kw_only() annotation
arg(): cannot specify an unnamed argument after a kw_only() annotation or args() argument
"""
)
@ -222,6 +269,47 @@ def test_positional_only_args(msg):
m.pos_only_def_mix(1, j=4)
assert "incompatible function arguments" in str(excinfo.value)
# Mix it with args and kwargs:
assert (
m.args_kwonly_full_monty.__doc__
== "args_kwonly_full_monty(arg0: int = 1, arg1: int = 2, /, j: float = 3.14159, *args, z: int = 42, **kwargs) -> tuple\n" # noqa: E501 line too long
)
assert m.args_kwonly_full_monty() == (1, 2, 3.14159, (), 42, {})
assert m.args_kwonly_full_monty(8) == (8, 2, 3.14159, (), 42, {})
assert m.args_kwonly_full_monty(8, 9) == (8, 9, 3.14159, (), 42, {})
assert m.args_kwonly_full_monty(8, 9, 10) == (8, 9, 10.0, (), 42, {})
assert m.args_kwonly_full_monty(3, 4, 5, 6, 7, m=8, z=9) == (
3,
4,
5.0,
(
6,
7,
),
9,
{"m": 8},
)
assert m.args_kwonly_full_monty(3, 4, 5, 6, 7, m=8, z=9) == (
3,
4,
5.0,
(
6,
7,
),
9,
{"m": 8},
)
assert m.args_kwonly_full_monty(5, j=7, m=8, z=9) == (5, 2, 7.0, (), 9, {"m": 8})
assert m.args_kwonly_full_monty(i=5, j=7, m=8, z=9) == (
1,
2,
7.0,
(),
9,
{"i": 5, "m": 8},
)
def test_signatures():
assert "kw_only_all(*, i: int, j: int) -> tuple\n" == m.kw_only_all.__doc__