From faaf1777e4c2f6f134b8e9a6f11f98d7aab28b86 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Wed, 17 Apr 2019 21:10:08 +0200 Subject: [PATCH 01/10] Fix the argument parsing TypeError message - parens are already added by the _LOCATION at call site - fix plural-"s" logic --- src/derive_utils.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/derive_utils.rs b/src/derive_utils.rs index cd9c9ca7..b578bd40 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -46,11 +46,10 @@ pub fn parse_fn_args<'p>( let nkeywords = kwargs.map_or(0, PyDict::len); if !accept_args && !accept_kwargs && (nargs + nkeywords > params.len()) { return Err(TypeError::py_err(format!( - "{}{} takes at most {} argument{} ({} given)", + "{} takes at most {} argument{} ({} given)", fname.unwrap_or("function"), - if fname.is_some() { "()" } else { "" }, params.len(), - if params.len() == 1 { "s" } else { "" }, + if params.len() == 1 { "" } else { "s" }, nargs + nkeywords ))); } From 3878bd7acd1c1f69dc03511d1ffdd5c4af410de0 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Wed, 17 Apr 2019 22:12:07 +0200 Subject: [PATCH 02/10] Adjust the varargs/kwds objects to remove arguments consumed by parameters Also fix some other validation issues and add more tests. fixes #420 --- CHANGELOG.md | 2 + pyo3-derive-backend/src/pymethod.rs | 7 ++- src/derive_utils.rs | 81 +++++++++++++++++------------ tests/test_methods.rs | 43 +++++++++++++++ 4 files changed, 97 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c26dc19e..45432633 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. the items are not hashable. * Fixed building using `venv` on Windows. * `PyTuple::new` now returns `&PyTuple` instead of `Py`. + * Fixed several issues with argument parsing; notable, the `*args` and `**kwargs` + tuple/dict now doesn't contain arguments that are otherwise assigned to parameters. ## [0.6.0] - 2018-03-28 diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index 994ab003..08d42f99 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -450,14 +450,17 @@ pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream { ]; let mut output = [#(#placeholders),*]; + let mut _args = _args; + let mut _kwargs = _kwargs; // Workaround to use the question mark operator without rewriting everything let _result = (|| { pyo3::derive_utils::parse_fn_args( + _py, Some(_LOCATION), PARAMS, - &_args, - _kwargs, + &mut _args, + &mut _kwargs, #accept_args, #accept_kwargs, &mut output diff --git a/src/derive_utils.rs b/src/derive_utils.rs index b578bd40..4d330b08 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -8,10 +8,10 @@ use crate::err::PyResult; use crate::exceptions::TypeError; use crate::ffi; use crate::init_once; -use crate::types::{PyAny, PyDict, PyModule, PyString, PyTuple}; +use crate::types::{PyAny, PyDict, PyModule, PyTuple}; use crate::GILPool; +use crate::IntoPyObject; use crate::Python; -use crate::{IntoPyObject, PyTryFrom}; use std::ptr; /// Description of a python parameter; used for `parse_args()`. @@ -34,75 +34,88 @@ pub struct ParamDescription { /// * output: Output array that receives the arguments. /// Must have same length as `params` and must be initialized to `None`. pub fn parse_fn_args<'p>( + py: Python<'p>, fname: Option<&str>, params: &[ParamDescription], - args: &'p PyTuple, - kwargs: Option<&'p PyDict>, + args: &mut &'p PyTuple, + kwargs: &mut Option<&'p PyDict>, accept_args: bool, accept_kwargs: bool, output: &mut [Option<&'p PyAny>], ) -> PyResult<()> { let nargs = args.len(); - let nkeywords = kwargs.map_or(0, PyDict::len); - if !accept_args && !accept_kwargs && (nargs + nkeywords > params.len()) { - return Err(TypeError::py_err(format!( - "{} takes at most {} argument{} ({} given)", - fname.unwrap_or("function"), - params.len(), - if params.len() == 1 { "" } else { "s" }, - nargs + nkeywords - ))); - } - let mut used_keywords = 0; + let mut used_args = 0; // Iterate through the parameters and assign values to output: for (i, (p, out)) in params.iter().zip(output).enumerate() { match kwargs.and_then(|d| d.get_item(p.name)) { Some(kwarg) => { *out = Some(kwarg); - used_keywords += 1; if i < nargs { return Err(TypeError::py_err(format!( - "Argument given by name ('{}') and position ({})", - p.name, - i + 1 + "{} got multiple values for argument '{}'", + fname.unwrap_or("function"), + p.name ))); } + kwargs.as_ref().unwrap().del_item(p.name).unwrap(); } None => { if p.kw_only { if !p.is_optional { return Err(TypeError::py_err(format!( - "Required argument ('{}') is keyword only argument", + "{} missing required keyword-only argument '{}'", + fname.unwrap_or("function"), p.name ))); } *out = None; } else if i < nargs { *out = Some(args.get_item(i)); + used_args += 1; } else { *out = None; if !p.is_optional { return Err(TypeError::py_err(format!( - "Required argument ('{}') (pos {}) not found", - p.name, - i + 1 + "{} missing required positional argument: '{}'", + fname.unwrap_or("function"), + p.name ))); } } } } } - if !accept_kwargs && used_keywords != nkeywords { - // check for extraneous keyword arguments - for item in kwargs.unwrap().items().iter() { - let item = ::try_from(item)?; - let key = ::try_from(item.get_item(0))?.to_string()?; - if !params.iter().any(|p| p.name == key) { - return Err(TypeError::py_err(format!( - "'{}' is an invalid keyword argument for this function", - key - ))); - } + // check for extraneous keyword arguments + if !accept_kwargs && !kwargs.as_ref().map_or(true, |d| d.is_empty()) { + for (key, _) in kwargs.unwrap().iter() { + // raise an error with any of the keys + return Err(TypeError::py_err(format!( + "{} got an unexpected keyword argument '{}'", + fname.unwrap_or("function"), + key + ))); + } + } + // check for extraneous positional arguments + if !accept_args && used_args < nargs { + return Err(TypeError::py_err(format!( + "{} takes at most {} positional argument{} ({} given)", + fname.unwrap_or("function"), + used_args, + if used_args == 1 { "" } else { "s" }, + nargs + ))); + } + // adjust the remaining args + if accept_args { + let slice = args + .slice(used_args as isize, nargs as isize) + .into_object(py); + *args = py.checked_cast_as(slice).unwrap(); + } + if accept_kwargs { + if kwargs.map_or(false, |d| d.is_empty()) { + *kwargs = None; } } Ok(()) diff --git a/tests/test_methods.rs b/tests/test_methods.rs index 0be9b7d3..e9efc002 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -232,6 +232,28 @@ impl MethArgs { ) -> PyResult { Ok([args.into(), kwargs.to_object(py)].to_object(py)) } + + #[args(args = "*", kwargs = "**")] + fn get_pos_arg_kw( + &self, + py: Python, + a: i32, + args: &PyTuple, + kwargs: Option<&PyDict>, + ) -> PyObject { + [a.to_object(py), args.into(), kwargs.to_object(py)].to_object(py) + } + + #[args(kwargs = "**")] + fn get_pos_kw(&self, py: Python, a: i32, kwargs: Option<&PyDict>) -> PyObject { + [a.to_object(py), kwargs.to_object(py)].to_object(py) + } + + // "args" can be anything that can be extracted from PyTuple + #[args(args = "*")] + fn args_as_vec(&self, args: Vec) -> i32 { + args.iter().sum() + } } #[test] @@ -259,6 +281,27 @@ fn meth_args() { inst, "assert inst.get_kwargs(1,2,3,t=1,n=2) == [(1,2,3), {'t': 1, 'n': 2}]" ); + + py_run!(py, inst, "assert inst.get_pos_arg_kw(1) == [1, (), None]"); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw(1, 2, 3) == [1, (2, 3), None]" + ); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw(1, b=2) == [1, (), {'b': 2}]" + ); + py_run!(py, inst, "assert inst.get_pos_arg_kw(a=1) == [1, (), None]"); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw()", TypeError); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw(1, a=1)", TypeError); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw(b=2)", TypeError); + + py_run!(py, inst, "assert inst.get_pos_kw(1, b=2) == [1, {'b': 2}]"); + py_expect_exception!(py, inst, "inst.get_pos_kw(1,2)", TypeError); + + py_run!(py, inst, "assert inst.args_as_vec(1,2,3) == 6"); } #[pyclass] From ab05a843b7389fee20aedc28182f4a8f994a4a44 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Thu, 18 Apr 2019 07:59:15 +0200 Subject: [PATCH 03/10] Document and test argument parsing annotations for pyfunctions --- guide/src/function.md | 28 ++++++++++++++++++++++++++++ tests/test_module.rs | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/guide/src/function.md b/guide/src/function.md index 1ead9a3b..f54ebeee 100644 --- a/guide/src/function.md +++ b/guide/src/function.md @@ -46,6 +46,34 @@ fn module_with_functions(py: Python, m: &PyModule) -> PyResult<()> { # fn main() {} ``` +## Argument parsing + +Both the `#[pyfunction]` and `#[pyfn]` attributes support specifying details of +argument parsing. The details are given in the section "Method arguments" in +the [Classes](class.md) chapter. Here is an example for a function that accepts +arbitrary keyword arguments (`**kwargs` in Python syntax) and returns the number +that was passed: + +```rust +# extern crate pyo3; +use pyo3::prelude::*; +use pyo3::wrap_pyfunction; +use pyo3::types::PyDict; + +#[pyfunction(kwds="**")] +fn num_kwds(kwds: Option<&PyDict>) -> usize { + kwds.map_or(0, |dict| dict.len()) +} + +#[pymodule] +fn module_with_functions(py: Python, m: &PyModule) -> PyResult<()> { + m.add_wrapped(wrap_pyfunction!(num_kwds)).unwrap(); + Ok(()) +} + +# fn main() {} +``` + ### Making the function signature available to Python In order to make the function signature available to Python to be retrieved via diff --git a/tests/test_module.rs b/tests/test_module.rs index 61b31907..d1bd8e64 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -1,6 +1,6 @@ use pyo3::prelude::*; -use pyo3::types::IntoPyDict; +use pyo3::types::{IntoPyDict, PyTuple}; #[macro_use] mod common; @@ -189,3 +189,35 @@ fn test_module_nesting() { "supermodule.submodule.subfunction() == 'Subfunction'" ); } + +// Test that argument parsing specification works for pyfunctions + +#[pyfunction(a = 5, vararg = "*")] +fn ext_vararg_fn(py: Python, a: i32, vararg: &PyTuple) -> PyObject { + [a.to_object(py), vararg.into()].to_object(py) +} + +#[pymodule] +fn vararg_module(_py: Python, m: &PyModule) -> PyResult<()> { + #[pyfn(m, "int_vararg_fn", a = 5, vararg = "*")] + fn int_vararg_fn(py: Python, a: i32, vararg: &PyTuple) -> PyObject { + ext_vararg_fn(py, a, vararg) + } + + m.add_wrapped(pyo3::wrap_pyfunction!(ext_vararg_fn)) + .unwrap(); + Ok(()) +} + +#[test] +fn test_vararg_module() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let m = pyo3::wrap_pymodule!(vararg_module)(py); + + py_assert!(py, m, "m.ext_vararg_fn() == [5, ()]"); + py_assert!(py, m, "m.ext_vararg_fn(1, 2) == [1, (2,)]"); + + py_assert!(py, m, "m.int_vararg_fn() == [5, ()]"); + py_assert!(py, m, "m.int_vararg_fn(1, 2) == [1, (2,)]"); +} From 7e6e4a923eb20235d2f71982ed4f16ac349b5d6a Mon Sep 17 00:00:00 2001 From: kngwyu Date: Sat, 25 May 2019 21:52:53 +0900 Subject: [PATCH 04/10] Refactor derive_utils --- src/derive_utils.rs | 65 +++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 4d330b08..ba6ab5d7 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -45,78 +45,63 @@ pub fn parse_fn_args<'p>( ) -> PyResult<()> { let nargs = args.len(); let mut used_args = 0; + macro_rules! raise_error { + ($s: expr $(,$arg:expr)*) => (return Err(TypeError::py_err(format!( + concat!("{} ", $s), fname.unwrap_or("function") $(,$arg)* + )))) + } // Iterate through the parameters and assign values to output: for (i, (p, out)) in params.iter().zip(output).enumerate() { - match kwargs.and_then(|d| d.get_item(p.name)) { + *out = match kwargs.and_then(|d| d.get_item(p.name)) { Some(kwarg) => { - *out = Some(kwarg); if i < nargs { - return Err(TypeError::py_err(format!( - "{} got multiple values for argument '{}'", - fname.unwrap_or("function"), - p.name - ))); + raise_error!("got multiple values for argument: {}", p.name) } kwargs.as_ref().unwrap().del_item(p.name).unwrap(); + Some(kwarg) } None => { if p.kw_only { if !p.is_optional { - return Err(TypeError::py_err(format!( - "{} missing required keyword-only argument '{}'", - fname.unwrap_or("function"), - p.name - ))); + raise_error!("missing required keyword-only argument: {}", p.name) } - *out = None; + None } else if i < nargs { - *out = Some(args.get_item(i)); used_args += 1; + Some(args.get_item(i)) } else { - *out = None; if !p.is_optional { - return Err(TypeError::py_err(format!( - "{} missing required positional argument: '{}'", - fname.unwrap_or("function"), - p.name - ))); + raise_error!("missing required positional argument: {}", p.name) } + None } } } } - // check for extraneous keyword arguments - if !accept_kwargs && !kwargs.as_ref().map_or(true, |d| d.is_empty()) { - for (key, _) in kwargs.unwrap().iter() { - // raise an error with any of the keys - return Err(TypeError::py_err(format!( - "{} got an unexpected keyword argument '{}'", - fname.unwrap_or("function"), - key - ))); - } + let is_kwargs_empty = kwargs.as_ref().map_or(true, |dict| dict.is_empty()); + // Raise an error when we get an unknown key + if !accept_kwargs && !is_kwargs_empty { + let (key, _) = kwargs.unwrap().iter().next().unwrap(); + raise_error!("got an unexpected keyword argument: {}", key) } - // check for extraneous positional arguments + // Raise an error when we get too many positional args if !accept_args && used_args < nargs { - return Err(TypeError::py_err(format!( - "{} takes at most {} positional argument{} ({} given)", - fname.unwrap_or("function"), + raise_error!( + "takes at most {} positional argument{} ({} given)", used_args, if used_args == 1 { "" } else { "s" }, nargs - ))); + ) } - // adjust the remaining args + // Adjust the remaining args if accept_args { let slice = args .slice(used_args as isize, nargs as isize) .into_object(py); *args = py.checked_cast_as(slice).unwrap(); } - if accept_kwargs { - if kwargs.map_or(false, |d| d.is_empty()) { - *kwargs = None; - } + if accept_kwargs && is_kwargs_empty { + *kwargs = None; } Ok(()) } From 241a8956c90cdc9c1e373bf5284a2ffcb5fa4ab9 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Wed, 17 Apr 2019 21:10:08 +0200 Subject: [PATCH 05/10] Fix the argument parsing TypeError message - parens are already added by the _LOCATION at call site - fix plural-"s" logic --- src/derive_utils.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/derive_utils.rs b/src/derive_utils.rs index cd9c9ca7..b578bd40 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -46,11 +46,10 @@ pub fn parse_fn_args<'p>( let nkeywords = kwargs.map_or(0, PyDict::len); if !accept_args && !accept_kwargs && (nargs + nkeywords > params.len()) { return Err(TypeError::py_err(format!( - "{}{} takes at most {} argument{} ({} given)", + "{} takes at most {} argument{} ({} given)", fname.unwrap_or("function"), - if fname.is_some() { "()" } else { "" }, params.len(), - if params.len() == 1 { "s" } else { "" }, + if params.len() == 1 { "" } else { "s" }, nargs + nkeywords ))); } From cba1657460d3e4d08dd1365a2187ccf3a19045a0 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Wed, 17 Apr 2019 22:12:07 +0200 Subject: [PATCH 06/10] Adjust the varargs/kwds objects to remove arguments consumed by parameters Also fix some other validation issues and add more tests. fixes #420 --- CHANGELOG.md | 2 + pyo3-derive-backend/src/pymethod.rs | 7 ++- src/derive_utils.rs | 81 +++++++++++++++++------------ tests/test_methods.rs | 43 +++++++++++++++ 4 files changed, 97 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45f08d7c..a0bcb078 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. the items are not hashable. * Fixed building using `venv` on Windows. * `PyTuple::new` now returns `&PyTuple` instead of `Py`. + * Fixed several issues with argument parsing; notable, the `*args` and `**kwargs` + tuple/dict now doesn't contain arguments that are otherwise assigned to parameters. ## [0.6.0] - 2018-03-28 diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index 994ab003..08d42f99 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -450,14 +450,17 @@ pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream { ]; let mut output = [#(#placeholders),*]; + let mut _args = _args; + let mut _kwargs = _kwargs; // Workaround to use the question mark operator without rewriting everything let _result = (|| { pyo3::derive_utils::parse_fn_args( + _py, Some(_LOCATION), PARAMS, - &_args, - _kwargs, + &mut _args, + &mut _kwargs, #accept_args, #accept_kwargs, &mut output diff --git a/src/derive_utils.rs b/src/derive_utils.rs index b578bd40..4d330b08 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -8,10 +8,10 @@ use crate::err::PyResult; use crate::exceptions::TypeError; use crate::ffi; use crate::init_once; -use crate::types::{PyAny, PyDict, PyModule, PyString, PyTuple}; +use crate::types::{PyAny, PyDict, PyModule, PyTuple}; use crate::GILPool; +use crate::IntoPyObject; use crate::Python; -use crate::{IntoPyObject, PyTryFrom}; use std::ptr; /// Description of a python parameter; used for `parse_args()`. @@ -34,75 +34,88 @@ pub struct ParamDescription { /// * output: Output array that receives the arguments. /// Must have same length as `params` and must be initialized to `None`. pub fn parse_fn_args<'p>( + py: Python<'p>, fname: Option<&str>, params: &[ParamDescription], - args: &'p PyTuple, - kwargs: Option<&'p PyDict>, + args: &mut &'p PyTuple, + kwargs: &mut Option<&'p PyDict>, accept_args: bool, accept_kwargs: bool, output: &mut [Option<&'p PyAny>], ) -> PyResult<()> { let nargs = args.len(); - let nkeywords = kwargs.map_or(0, PyDict::len); - if !accept_args && !accept_kwargs && (nargs + nkeywords > params.len()) { - return Err(TypeError::py_err(format!( - "{} takes at most {} argument{} ({} given)", - fname.unwrap_or("function"), - params.len(), - if params.len() == 1 { "" } else { "s" }, - nargs + nkeywords - ))); - } - let mut used_keywords = 0; + let mut used_args = 0; // Iterate through the parameters and assign values to output: for (i, (p, out)) in params.iter().zip(output).enumerate() { match kwargs.and_then(|d| d.get_item(p.name)) { Some(kwarg) => { *out = Some(kwarg); - used_keywords += 1; if i < nargs { return Err(TypeError::py_err(format!( - "Argument given by name ('{}') and position ({})", - p.name, - i + 1 + "{} got multiple values for argument '{}'", + fname.unwrap_or("function"), + p.name ))); } + kwargs.as_ref().unwrap().del_item(p.name).unwrap(); } None => { if p.kw_only { if !p.is_optional { return Err(TypeError::py_err(format!( - "Required argument ('{}') is keyword only argument", + "{} missing required keyword-only argument '{}'", + fname.unwrap_or("function"), p.name ))); } *out = None; } else if i < nargs { *out = Some(args.get_item(i)); + used_args += 1; } else { *out = None; if !p.is_optional { return Err(TypeError::py_err(format!( - "Required argument ('{}') (pos {}) not found", - p.name, - i + 1 + "{} missing required positional argument: '{}'", + fname.unwrap_or("function"), + p.name ))); } } } } } - if !accept_kwargs && used_keywords != nkeywords { - // check for extraneous keyword arguments - for item in kwargs.unwrap().items().iter() { - let item = ::try_from(item)?; - let key = ::try_from(item.get_item(0))?.to_string()?; - if !params.iter().any(|p| p.name == key) { - return Err(TypeError::py_err(format!( - "'{}' is an invalid keyword argument for this function", - key - ))); - } + // check for extraneous keyword arguments + if !accept_kwargs && !kwargs.as_ref().map_or(true, |d| d.is_empty()) { + for (key, _) in kwargs.unwrap().iter() { + // raise an error with any of the keys + return Err(TypeError::py_err(format!( + "{} got an unexpected keyword argument '{}'", + fname.unwrap_or("function"), + key + ))); + } + } + // check for extraneous positional arguments + if !accept_args && used_args < nargs { + return Err(TypeError::py_err(format!( + "{} takes at most {} positional argument{} ({} given)", + fname.unwrap_or("function"), + used_args, + if used_args == 1 { "" } else { "s" }, + nargs + ))); + } + // adjust the remaining args + if accept_args { + let slice = args + .slice(used_args as isize, nargs as isize) + .into_object(py); + *args = py.checked_cast_as(slice).unwrap(); + } + if accept_kwargs { + if kwargs.map_or(false, |d| d.is_empty()) { + *kwargs = None; } } Ok(()) diff --git a/tests/test_methods.rs b/tests/test_methods.rs index 21625edb..5906ba64 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -232,6 +232,28 @@ impl MethArgs { ) -> PyResult { Ok([args.into(), kwargs.to_object(py)].to_object(py)) } + + #[args(args = "*", kwargs = "**")] + fn get_pos_arg_kw( + &self, + py: Python, + a: i32, + args: &PyTuple, + kwargs: Option<&PyDict>, + ) -> PyObject { + [a.to_object(py), args.into(), kwargs.to_object(py)].to_object(py) + } + + #[args(kwargs = "**")] + fn get_pos_kw(&self, py: Python, a: i32, kwargs: Option<&PyDict>) -> PyObject { + [a.to_object(py), kwargs.to_object(py)].to_object(py) + } + + // "args" can be anything that can be extracted from PyTuple + #[args(args = "*")] + fn args_as_vec(&self, args: Vec) -> i32 { + args.iter().sum() + } } #[test] @@ -259,6 +281,27 @@ fn meth_args() { inst, "assert inst.get_kwargs(1,2,3,t=1,n=2) == [(1,2,3), {'t': 1, 'n': 2}]" ); + + py_run!(py, inst, "assert inst.get_pos_arg_kw(1) == [1, (), None]"); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw(1, 2, 3) == [1, (2, 3), None]" + ); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw(1, b=2) == [1, (), {'b': 2}]" + ); + py_run!(py, inst, "assert inst.get_pos_arg_kw(a=1) == [1, (), None]"); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw()", TypeError); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw(1, a=1)", TypeError); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw(b=2)", TypeError); + + py_run!(py, inst, "assert inst.get_pos_kw(1, b=2) == [1, {'b': 2}]"); + py_expect_exception!(py, inst, "inst.get_pos_kw(1,2)", TypeError); + + py_run!(py, inst, "assert inst.args_as_vec(1,2,3) == 6"); } #[pyclass] From ab802cd829aebf65f3a001fcc5c0df822d252e27 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Thu, 18 Apr 2019 07:59:15 +0200 Subject: [PATCH 07/10] Document and test argument parsing annotations for pyfunctions --- guide/src/function.md | 28 ++++++++++++++++++++++++++++ tests/test_module.rs | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/guide/src/function.md b/guide/src/function.md index 1ead9a3b..f54ebeee 100644 --- a/guide/src/function.md +++ b/guide/src/function.md @@ -46,6 +46,34 @@ fn module_with_functions(py: Python, m: &PyModule) -> PyResult<()> { # fn main() {} ``` +## Argument parsing + +Both the `#[pyfunction]` and `#[pyfn]` attributes support specifying details of +argument parsing. The details are given in the section "Method arguments" in +the [Classes](class.md) chapter. Here is an example for a function that accepts +arbitrary keyword arguments (`**kwargs` in Python syntax) and returns the number +that was passed: + +```rust +# extern crate pyo3; +use pyo3::prelude::*; +use pyo3::wrap_pyfunction; +use pyo3::types::PyDict; + +#[pyfunction(kwds="**")] +fn num_kwds(kwds: Option<&PyDict>) -> usize { + kwds.map_or(0, |dict| dict.len()) +} + +#[pymodule] +fn module_with_functions(py: Python, m: &PyModule) -> PyResult<()> { + m.add_wrapped(wrap_pyfunction!(num_kwds)).unwrap(); + Ok(()) +} + +# fn main() {} +``` + ### Making the function signature available to Python In order to make the function signature available to Python to be retrieved via diff --git a/tests/test_module.rs b/tests/test_module.rs index edc6e5f8..cda0d835 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -1,6 +1,6 @@ use pyo3::prelude::*; -use pyo3::types::IntoPyDict; +use pyo3::types::{IntoPyDict, PyTuple}; mod common; @@ -194,3 +194,35 @@ fn test_module_nesting() { "supermodule.submodule.subfunction() == 'Subfunction'" ); } + +// Test that argument parsing specification works for pyfunctions + +#[pyfunction(a = 5, vararg = "*")] +fn ext_vararg_fn(py: Python, a: i32, vararg: &PyTuple) -> PyObject { + [a.to_object(py), vararg.into()].to_object(py) +} + +#[pymodule] +fn vararg_module(_py: Python, m: &PyModule) -> PyResult<()> { + #[pyfn(m, "int_vararg_fn", a = 5, vararg = "*")] + fn int_vararg_fn(py: Python, a: i32, vararg: &PyTuple) -> PyObject { + ext_vararg_fn(py, a, vararg) + } + + m.add_wrapped(pyo3::wrap_pyfunction!(ext_vararg_fn)) + .unwrap(); + Ok(()) +} + +#[test] +fn test_vararg_module() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let m = pyo3::wrap_pymodule!(vararg_module)(py); + + py_assert!(py, m, "m.ext_vararg_fn() == [5, ()]"); + py_assert!(py, m, "m.ext_vararg_fn(1, 2) == [1, (2,)]"); + + py_assert!(py, m, "m.int_vararg_fn() == [5, ()]"); + py_assert!(py, m, "m.int_vararg_fn(1, 2) == [1, (2,)]"); +} From a4cf09f16afc282513ba4e42aa26ec20f5597045 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Sat, 25 May 2019 21:52:53 +0900 Subject: [PATCH 08/10] Refactor derive_utils --- src/derive_utils.rs | 65 +++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 4d330b08..ba6ab5d7 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -45,78 +45,63 @@ pub fn parse_fn_args<'p>( ) -> PyResult<()> { let nargs = args.len(); let mut used_args = 0; + macro_rules! raise_error { + ($s: expr $(,$arg:expr)*) => (return Err(TypeError::py_err(format!( + concat!("{} ", $s), fname.unwrap_or("function") $(,$arg)* + )))) + } // Iterate through the parameters and assign values to output: for (i, (p, out)) in params.iter().zip(output).enumerate() { - match kwargs.and_then(|d| d.get_item(p.name)) { + *out = match kwargs.and_then(|d| d.get_item(p.name)) { Some(kwarg) => { - *out = Some(kwarg); if i < nargs { - return Err(TypeError::py_err(format!( - "{} got multiple values for argument '{}'", - fname.unwrap_or("function"), - p.name - ))); + raise_error!("got multiple values for argument: {}", p.name) } kwargs.as_ref().unwrap().del_item(p.name).unwrap(); + Some(kwarg) } None => { if p.kw_only { if !p.is_optional { - return Err(TypeError::py_err(format!( - "{} missing required keyword-only argument '{}'", - fname.unwrap_or("function"), - p.name - ))); + raise_error!("missing required keyword-only argument: {}", p.name) } - *out = None; + None } else if i < nargs { - *out = Some(args.get_item(i)); used_args += 1; + Some(args.get_item(i)) } else { - *out = None; if !p.is_optional { - return Err(TypeError::py_err(format!( - "{} missing required positional argument: '{}'", - fname.unwrap_or("function"), - p.name - ))); + raise_error!("missing required positional argument: {}", p.name) } + None } } } } - // check for extraneous keyword arguments - if !accept_kwargs && !kwargs.as_ref().map_or(true, |d| d.is_empty()) { - for (key, _) in kwargs.unwrap().iter() { - // raise an error with any of the keys - return Err(TypeError::py_err(format!( - "{} got an unexpected keyword argument '{}'", - fname.unwrap_or("function"), - key - ))); - } + let is_kwargs_empty = kwargs.as_ref().map_or(true, |dict| dict.is_empty()); + // Raise an error when we get an unknown key + if !accept_kwargs && !is_kwargs_empty { + let (key, _) = kwargs.unwrap().iter().next().unwrap(); + raise_error!("got an unexpected keyword argument: {}", key) } - // check for extraneous positional arguments + // Raise an error when we get too many positional args if !accept_args && used_args < nargs { - return Err(TypeError::py_err(format!( - "{} takes at most {} positional argument{} ({} given)", - fname.unwrap_or("function"), + raise_error!( + "takes at most {} positional argument{} ({} given)", used_args, if used_args == 1 { "" } else { "s" }, nargs - ))); + ) } - // adjust the remaining args + // Adjust the remaining args if accept_args { let slice = args .slice(used_args as isize, nargs as isize) .into_object(py); *args = py.checked_cast_as(slice).unwrap(); } - if accept_kwargs { - if kwargs.map_or(false, |d| d.is_empty()) { - *kwargs = None; - } + if accept_kwargs && is_kwargs_empty { + *kwargs = None; } Ok(()) } From c7e377a472c0e6a057286bef42196de2f47d0390 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Sun, 1 Sep 2019 23:59:24 +0900 Subject: [PATCH 09/10] [derive_utils] Copy kwargs not to modify it --- pyo3-derive-backend/src/pymethod.rs | 6 +++--- src/derive_utils.rs | 33 +++++++++++++++++------------ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index c4bf3720..6d1e93d1 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -475,12 +475,12 @@ pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream { // Workaround to use the question mark operator without rewriting everything let _result = (|| { - pyo3::derive_utils::parse_fn_args( + let (_args, _kwargs) = pyo3::derive_utils::parse_fn_args( _py, Some(_LOCATION), PARAMS, - &mut _args, - &mut _kwargs, + _args, + _kwargs, #accept_args, #accept_kwargs, &mut output diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 6500c5a7..577e8d6c 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -36,12 +36,12 @@ pub fn parse_fn_args<'p>( py: Python<'p>, fname: Option<&str>, params: &[ParamDescription], - args: &mut &'p PyTuple, - kwargs: &mut Option<&'p PyDict>, + args: &'p PyTuple, + kwargs: Option<&'p PyDict>, accept_args: bool, accept_kwargs: bool, output: &mut [Option<&'p PyAny>], -) -> PyResult<()> { +) -> PyResult<(&'p PyTuple, Option<&'p PyDict>)> { let nargs = args.len(); let mut used_args = 0; macro_rules! raise_error { @@ -49,6 +49,11 @@ pub fn parse_fn_args<'p>( concat!("{} ", $s), fname.unwrap_or("function") $(,$arg)* )))) } + // Copy kwargs not to modify it + let kwargs = match kwargs { + Some(k) => Some(k.copy()?), + None => None, + }; // Iterate through the parameters and assign values to output: for (i, (p, out)) in params.iter().zip(output).enumerate() { *out = match kwargs.and_then(|d| d.get_item(p.name)) { @@ -93,16 +98,18 @@ pub fn parse_fn_args<'p>( ) } // Adjust the remaining args - if accept_args { - let slice = args - .slice(used_args as isize, nargs as isize) - .into_object(py); - *args = py.checked_cast_as(slice).unwrap(); - } - if accept_kwargs && is_kwargs_empty { - *kwargs = None; - } - Ok(()) + let args = if accept_args { + let slice = args.slice(used_args as isize, nargs as isize).into_py(py); + py.checked_cast_as(slice).unwrap() + } else { + args + }; + let kwargs = if accept_kwargs && is_kwargs_empty { + None + } else { + kwargs + }; + Ok((args, kwargs)) } /// Builds a module (or null) from a user given initializer. Used for `#[pymodule]`. From df44e500a9b64c6ac2549b566f1a60e431488f61 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Mon, 2 Sep 2019 23:05:42 +0900 Subject: [PATCH 10/10] Remove py from parse_fn_args's args(to address clippy warning) --- pyo3-derive-backend/src/pymethod.rs | 1 - src/derive_utils.rs | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index 6d1e93d1..9b9a4ef1 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -476,7 +476,6 @@ pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream { // Workaround to use the question mark operator without rewriting everything let _result = (|| { let (_args, _kwargs) = pyo3::derive_utils::parse_fn_args( - _py, Some(_LOCATION), PARAMS, _args, diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 577e8d6c..171a6405 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -7,6 +7,7 @@ use crate::err::PyResult; use crate::exceptions::TypeError; use crate::init_once; +use crate::instance::PyNativeType; use crate::types::{PyAny, PyDict, PyModule, PyTuple}; use crate::GILPool; use crate::Python; @@ -33,7 +34,6 @@ pub struct ParamDescription { /// * output: Output array that receives the arguments. /// Must have same length as `params` and must be initialized to `None`. pub fn parse_fn_args<'p>( - py: Python<'p>, fname: Option<&str>, params: &[ParamDescription], args: &'p PyTuple, @@ -99,6 +99,7 @@ pub fn parse_fn_args<'p>( } // Adjust the remaining args let args = if accept_args { + let py = args.py(); let slice = args.slice(used_args as isize, nargs as isize).into_py(py); py.checked_cast_as(slice).unwrap() } else {