-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: py::pos_only #2459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: py::pos_only #2459
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -187,11 +187,13 @@ class cpp_function : public function { | |
process_attributes<Extra...>::init(extra..., rec); | ||
|
||
{ | ||
constexpr bool has_kwonly_args = any_of<std::is_same<kwonly, Extra>...>::value, | ||
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_kwonly_args, "py::kwonly requires the use of argument annotations"); | ||
static_assert(!(has_args && has_kwonly_args), "py::kwonly cannot be combined with a py::args argument"); | ||
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"); | ||
} | ||
|
||
/* Generate a readable signature describing the function's arguments and return value types */ | ||
|
@@ -257,7 +259,10 @@ class cpp_function : public function { | |
// Write arg name for everything except *args and **kwargs. | ||
if (*(pc + 1) == '*') | ||
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) | ||
signature += "*, "; | ||
henryiii marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (arg_index < rec->args.size() && rec->args[arg_index].name) { | ||
signature += rec->args[arg_index].name; | ||
} else if (arg_index == 0 && rec->is_method) { | ||
|
@@ -272,6 +277,10 @@ class cpp_function : public function { | |
signature += " = "; | ||
signature += rec->args[arg_index].descr; | ||
} | ||
// Separator for positional-only arguments (placed after the | ||
// argument, rather than before like * | ||
if (rec->nargs_pos_only > 0 && (arg_index + 1) == rec->nargs_pos_only) | ||
signature += ", /"; | ||
arg_index++; | ||
} else if (c == '%') { | ||
const std::type_info *t = types[type_index++]; | ||
|
@@ -297,6 +306,7 @@ class cpp_function : public function { | |
signature += c; | ||
} | ||
} | ||
|
||
if (arg_index != args || types[type_index] != nullptr) | ||
pybind11_fail("Internal error while parsing type signature (2)"); | ||
|
||
|
@@ -512,7 +522,7 @@ class cpp_function : public function { | |
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_kwonly; | ||
size_t pos_args = num_args - func.nargs_kw_only; | ||
|
||
if (!func.has_args && n_args_in > pos_args) | ||
continue; // Too many positional arguments for this overload | ||
|
@@ -561,6 +571,26 @@ class cpp_function : public function { | |
// We'll need to copy this if we steal some kwargs for defaults | ||
dict kwargs = reinterpret_borrow<dict>(kwargs_in); | ||
|
||
// 1.5. Fill in any missing pos_only args from defaults if they exist | ||
if (args_copied < func.nargs_pos_only) { | ||
for (; args_copied < func.nargs_pos_only; ++args_copied) { | ||
const auto &arg = func.args[args_copied]; | ||
handle value; | ||
|
||
if (arg.value) { | ||
value = arg.value; | ||
} | ||
if (value) { | ||
call.args.push_back(value); | ||
call.args_convert.push_back(arg.convert); | ||
} else | ||
break; | ||
YannickJadoul marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't checked the rest of the codebase, so if this is consistent, leave it. I'm just a little bothered by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's done the same way just below, in the keyword checking. |
||
} | ||
|
||
if (args_copied < func.nargs_pos_only) | ||
continue; // Not enough defaults to fill the positional arguments | ||
} | ||
|
||
// 2. Check kwargs and, failing that, defaults that may help complete the list | ||
if (args_copied < num_args) { | ||
bool copied_kwargs = false; | ||
|
Uh oh!
There was an error while loading. Please reload this page.