luxolus
/
pyo3
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Positional-only args (#1925) 

* Add support for positional-only args

* Update changelog. Add a few more tests. Run rust-fmt.

* Fix test.

* Fix tests again.

* Update CHANGELOG.md to link PR instead of issue

* Update guide to mention positional-only params

* Add unreachable!() per code review

* Update and expand tests for pos args.

* Fix tests, lint, add UI tests.

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
This commit is contained in:
Ashley Anderson 2021-10-19 18:13:27 -04:00 committed by GitHub
parent 9d846c3ddb
commit bf26daec2d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 292 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add `as_sequence` methods to `PyList` and `PyTuple`. [#1860](https://github.com/PyO3/pyo3/pull/1860)
- Add `abi3-py310` feature. [#1889](https://github.com/PyO3/pyo3/pull/1889)
- Add `PyCFunction::new_closure` to create a Python function from a Rust closure. [#1901](https://github.com/PyO3/pyo3/pull/1901)
- Add support for positional-only arguments in `#[pyfunction]` [#1925](https://github.com/PyO3/pyo3/pull/1925)
### Changed

5
guide/src/class.md
View File

@ -691,6 +691,9 @@ the form of `attr_name="default value"`. Each parameter has to match the method
Each parameter can be one of the following types:
* `"/"`: positional-only arguments separator, each parameter defined before `"/"` is a
positional-only parameter.
Corresponds to python's `def meth(arg1, arg2, ..., /, argN..)`.
* `"*"`: var arguments separator, each parameter defined after `"*"` is a keyword-only parameter.
Corresponds to python's `def meth(*, arg1.., arg2=..)`.
* `args="*"`: "args" is var args, corresponds to Python's `def meth(*args)`. Type of the `args`
@ -745,7 +748,7 @@ impl MyClass {
}
}
```
N.B. the position of the `"*"` argument (if included) controls the system of handling positional and keyword arguments. In Python:
N.B. the position of the `"/"` and `"*"` arguments (if included) control the system of handling positional and keyword arguments. In Python:
```python
import mymodule

11
pyo3-macros-backend/src/method.rs
View File

@ -449,6 +449,17 @@ impl<'a> FnSpec<'a> {
None
}
pub fn is_pos_only(&self, name: &syn::Ident) -> bool {
for s in self.attrs.iter() {
if let Argument::PosOnlyArg(path, _) = s {
if path.is_ident(name) {
return true;
}
}
}
false
}
pub fn is_kw_only(&self, name: &syn::Ident) -> bool {
for s in self.attrs.iter() {
if let Argument::Kwarg(path, _) = s {

8
pyo3-macros-backend/src/params.rs
View File

@ -78,6 +78,7 @@ pub fn impl_arg_params(
};
let mut positional_parameter_names = Vec::new();
let mut positional_only_parameters = 0usize;
let mut required_positional_parameters = 0usize;
let mut keyword_only_parameters = Vec::new();
@ -86,6 +87,7 @@ pub fn impl_arg_params(
continue;
}
let name = arg.name.unraw().to_string();
let posonly = spec.is_pos_only(arg.name);
let kwonly = spec.is_kw_only(arg.name);
let required = !(arg.optional.is_some() || spec.default_value(arg.name).is_some());
@ -100,6 +102,9 @@ pub fn impl_arg_params(
if required {
required_positional_parameters += 1;
}
if posonly {
positional_only_parameters += 1;
}
positional_parameter_names.push(name);
}
}
@ -154,8 +159,7 @@ pub fn impl_arg_params(
cls_name: #cls_name,
func_name: stringify!(#python_name),
positional_parameter_names: &[#(#positional_parameter_names),*],
// TODO: https://github.com/PyO3/pyo3/issues/1439 - support specifying these
positional_only_parameters: 0,
positional_only_parameters: #positional_only_parameters,
required_positional_parameters: #required_positional_parameters,
keyword_only_parameters: &[#(#keyword_only_parameters),*],
accept_varargs: #accept_args,

28
pyo3-macros-backend/src/pyfunction.rs
View File

@ -22,9 +22,11 @@ use syn::{
#[derive(Debug, Clone, PartialEq)]
pub enum Argument {
PosOnlyArgsSeparator,
VarArgsSeparator,
VarArgs(syn::Path),
KeywordArgs(syn::Path),
PosOnlyArg(syn::Path, Option<String>),
Arg(syn::Path, Option<String>),
Kwarg(syn::Path, Option<String>),
}
@ -34,6 +36,7 @@ pub enum Argument {
pub struct PyFunctionSignature {
pub arguments: Vec<Argument>,
has_kw: bool,
has_posonly_args: bool,
has_varargs: bool,
has_kwargs: bool,
}
@ -126,7 +129,22 @@ impl PyFunctionSignature {
self.arguments.push(Argument::VarArgsSeparator);
Ok(())
}
_ => bail_spanned!(item.span() => "expected \"*\""),
syn::Lit::Str(lits) if lits.value() == "/" => {
// "/"
self.posonly_arg_is_ok(item)?;
self.has_posonly_args = true;
// any arguments _before_ this become positional-only
self.arguments.iter_mut().for_each(|a| {
if let Argument::Arg(path, name) = a {
*a = Argument::PosOnlyArg(path.clone(), name.clone());
} else {
unreachable!();
}
});
self.arguments.push(Argument::PosOnlyArgsSeparator);
Ok(())
}
_ => bail_spanned!(item.span() => "expected \"/\" or \"*\""),
}
}
@ -143,6 +161,14 @@ impl PyFunctionSignature {
Ok(())
}
fn posonly_arg_is_ok(&self, item: &NestedMeta) -> syn::Result<()> {
ensure_spanned!(
!(self.has_posonly_args || self.has_kwargs || self.has_varargs),
item.span() => "/ is not allowed after /, varargs(*), or kwargs(**)"
);
Ok(())
}
fn vararg_is_ok(&self, item: &NestedMeta) -> syn::Result<()> {
ensure_spanned!(
!(self.has_kwargs || self.has_varargs),

199
tests/test_methods.rs
View File

@ -215,6 +215,46 @@ impl MethArgs {
[a.to_object(py), args.into(), kwargs.to_object(py)].to_object(py)
}
#[args(a, b, "/")]
fn get_pos_only(&self, a: i32, b: i32) -> i32 {
a + b
}
#[args(a, "/", b)]
fn get_pos_only_and_pos(&self, a: i32, b: i32) -> i32 {
a + b
}
#[args(a, "/", b, c = 5)]
fn get_pos_only_and_pos_and_kw(&self, a: i32, b: i32, c: i32) -> i32 {
a + b + c
}
#[args(a, "/", "*", b)]
fn get_pos_only_and_kw_only(&self, a: i32, b: i32) -> i32 {
a + b
}
#[args(a, "/", "*", b = 3)]
fn get_pos_only_and_kw_only_with_default(&self, a: i32, b: i32) -> i32 {
a + b
}
#[args(a, "/", b, "*", c, d = 5)]
fn get_all_arg_types_together(&self, a: i32, b: i32, c: i32, d: i32) -> i32 {
a + b + c + d
}
#[args(a, "/", args = "*")]
fn get_pos_only_with_varargs(&self, a: i32, args: Vec<i32>) -> i32 {
a + args.iter().sum::<i32>()
}
#[args(a, "/", kwargs = "**")]
fn get_pos_only_with_kwargs(&self, py: Python, a: i32, kwargs: Option<&PyDict>) -> PyObject {
[a.to_object(py), kwargs.to_object(py)].to_object(py)
}
#[args("*", a = 2, b = 3)]
fn get_kwargs_only_with_defaults(&self, a: i32, b: i32) -> i32 {
a + b
@ -308,6 +348,165 @@ fn meth_args() {
py_expect_exception!(py, inst, "inst.get_pos_arg_kw(1, a=1)", PyTypeError);
py_expect_exception!(py, inst, "inst.get_pos_arg_kw(b=2)", PyTypeError);
py_run!(py, inst, "assert inst.get_pos_only(10, 11) == 21");
py_expect_exception!(py, inst, "inst.get_pos_only(10, b = 11)", PyTypeError);
py_expect_exception!(py, inst, "inst.get_pos_only(a = 10, b = 11)", PyTypeError);
py_run!(py, inst, "assert inst.get_pos_only_and_pos(10, 11) == 21");
py_run!(
py,
inst,
"assert inst.get_pos_only_and_pos(10, b = 11) == 21"
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_and_pos(a = 10, b = 11)",
PyTypeError
);
py_run!(
py,
inst,
"assert inst.get_pos_only_and_pos_and_kw(10, 11) == 26"
);
py_run!(
py,
inst,
"assert inst.get_pos_only_and_pos_and_kw(10, b = 11) == 26"
);
py_run!(
py,
inst,
"assert inst.get_pos_only_and_pos_and_kw(10, 11, c = 0) == 21"
);
py_run!(
py,
inst,
"assert inst.get_pos_only_and_pos_and_kw(10, b = 11, c = 0) == 21"
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_and_pos_and_kw(a = 10, b = 11)",
PyTypeError
);
py_run!(
py,
inst,
"assert inst.get_pos_only_and_kw_only(10, b = 11) == 21"
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_and_kw_only(10, 11)",
PyTypeError
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_and_kw_only(a = 10, b = 11)",
PyTypeError
);
py_run!(
py,
inst,
"assert inst.get_pos_only_and_kw_only_with_default(10) == 13"
);
py_run!(
py,
inst,
"assert inst.get_pos_only_and_kw_only_with_default(10, b = 11) == 21"
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_and_kw_only_with_default(10, 11)",
PyTypeError
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_and_kw_only_with_default(a = 10, b = 11)",
PyTypeError
);
py_run!(
py,
inst,
"assert inst.get_all_arg_types_together(10, 10, c = 10) == 35"
);
py_run!(
py,
inst,
"assert inst.get_all_arg_types_together(10, 10, c = 10, d = 10) == 40"
);
py_run!(
py,
inst,
"assert inst.get_all_arg_types_together(10, b = 10, c = 10, d = 10) == 40"
);
py_expect_exception!(
py,
inst,
"inst.get_all_arg_types_together(10, 10, 10)",
PyTypeError
);
py_expect_exception!(
py,
inst,
"inst.get_all_arg_types_together(a = 10, b = 10, c = 10)",
PyTypeError
);
py_run!(py, inst, "assert inst.get_pos_only_with_varargs(10) == 10");
py_run!(
py,
inst,
"assert inst.get_pos_only_with_varargs(10, 10) == 20"
);
py_run!(
py,
inst,
"assert inst.get_pos_only_with_varargs(10, 10, 10, 10, 10) == 50"
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_with_varargs(a = 10)",
PyTypeError
);
py_run!(
py,
inst,
"assert inst.get_pos_only_with_kwargs(10) == [10, None]"
);
py_run!(
py,
inst,
"assert inst.get_pos_only_with_kwargs(10, b = 10) == [10, {'b': 10}]"
);
py_run!(
py,
inst,
"assert inst.get_pos_only_with_kwargs(10, b = 10, c = 10, d = 10, e = 10) == [10, {'b': 10, 'c': 10, 'd': 10, 'e': 10}]"
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_with_kwargs(a = 10)",
PyTypeError
);
py_expect_exception!(
py,
inst,
"inst.get_pos_only_with_kwargs(a = 10, b = 10)",
PyTypeError
);
py_run!(py, inst, "assert inst.get_kwargs_only_with_defaults() == 5");
py_run!(
py,

20
tests/ui/invalid_macro_args.rs
View File

@ -10,4 +10,24 @@ fn kw_after_kwargs(py: Python, kwargs: &PyDict, a: i32) -> PyObject {
[a.to_object(py), vararg.into()].to_object(py)
}
#[pyfunction(a, "*", b, "/", c)]
fn pos_only_after_kw_only(py: Python, a: i32, b: i32, c: i32) -> i32 {
a + b + c
}
#[pyfunction(a, args="*", "/", b)]
fn pos_only_after_args(py: Python, a: i32, args: Vec<i32>, b: i32) -> i32 {
a + b + c
}
#[pyfunction(a, kwargs="**", "/", b)]
fn pos_only_after_kwargs(py: Python, a: i32, args: Vec<i32>, b: i32) -> i32 {
a + b
}
#[pyfunction(kwargs = "**", "*", a)]
fn kw_only_after_kwargs(py: Python, kwargs: &PyDict, a: i32) -> PyObject {
[a.to_object(py), vararg.into()].to_object(py)
}
fn main() {}

24
tests/ui/invalid_macro_args.stderr
View File

@ -9,3 +9,27 @@ error: keyword argument or kwargs(**) is not allowed after kwargs(**)
|
8 | #[pyfunction(kwargs = "**", a = 5)]
| ^
error: / is not allowed after /, varargs(*), or kwargs(**)
--> tests/ui/invalid_macro_args.rs:13:25
|
13 | #[pyfunction(a, "*", b, "/", c)]
| ^^^
error: / is not allowed after /, varargs(*), or kwargs(**)
--> tests/ui/invalid_macro_args.rs:18:27
|
18 | #[pyfunction(a, args="*", "/", b)]
| ^^^
error: / is not allowed after /, varargs(*), or kwargs(**)
--> tests/ui/invalid_macro_args.rs:23:30
|
23 | #[pyfunction(a, kwargs="**", "/", b)]
| ^^^
error: * is not allowed after varargs(*) or kwargs(**)
--> tests/ui/invalid_macro_args.rs:28:29
|
28 | #[pyfunction(kwargs = "**", "*", a)]
| ^^^