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

pyfunction: fix args conflicting with keyword only arg

This commit is contained in:
David Hewitt 2021-02-20 09:07:51 +00:00
parent f9ad119871
commit ffd5874c3a
4 changed files with 53 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- `PYO3_CROSS_LIB_DIR` enviroment variable no long required when compiling for x86-64 Python from macOS arm64 and reverse. [#1428](https://github.com/PyO3/pyo3/pull/1428)
- Fix FFI definition `_PyEval_RequestCodeExtraIndex` which took an argument of the wrong type. [#1429](https://github.com/PyO3/pyo3/pull/1429)
- Fix FFI definition `PyIndex_Check` missing with the `abi3` feature. [#1436](https://github.com/PyO3/pyo3/pull/1436)
- Fix incorrect `TypeError` raised when keyword-only argument passed along with a positional argument in `*args`. [#1440](https://github.com/PyO3/pyo3/pull/1440)
## [0.13.2] - 2021-02-12
### Packaging

15
pyo3-macros-backend/src/pymethod.rs
View File

@ -407,6 +407,7 @@ pub fn impl_arg_params(
}
let mut params = Vec::new();
let mut num_positional_params = 0usize;
for arg in spec.args.iter() {
if arg.py || spec.is_args(&arg.name) || spec.is_kwargs(&arg.name) {
@ -416,6 +417,10 @@ pub fn impl_arg_params(
let kwonly = spec.is_kw_only(&arg.name);
let opt = arg.optional.is_some() || spec.default_value(&arg.name).is_some();
if !kwonly {
num_positional_params += 1;
}
params.push(quote! {
pyo3::derive_utils::ParamDescription {
name: #name,
@ -425,6 +430,8 @@ pub fn impl_arg_params(
});
}
let num_normal_params = params.len();
let mut param_conversion = Vec::new();
let mut option_pos = 0;
for (idx, arg) in spec.args.iter().enumerate() {
@ -441,7 +448,7 @@ pub fn impl_arg_params(
_ => continue,
}
}
let num_normal_params = params.len();
// create array of arguments, and then parse
quote! {{
const PARAMS: &'static [pyo3::derive_utils::ParamDescription] = &[
@ -449,14 +456,12 @@ pub fn impl_arg_params(
];
let mut output = [None; #num_normal_params];
let mut _args = _args;
let mut _kwargs = _kwargs;
let (_args, _kwargs) = pyo3::derive_utils::parse_fn_args(
Some(_LOCATION),
_LOCATION,
PARAMS,
_args,
_kwargs,
#num_positional_params,
#accept_args,
#accept_kwargs,
&mut output

35
src/derive_utils.rs
View File

@ -33,21 +33,24 @@ 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>(
fname: Option<&str>,
fname: &str,
params: &[ParamDescription],
args: &'p PyTuple,
kwargs: Option<&'p PyDict>,
num_positional_params: usize,
accept_args: bool,
accept_kwargs: bool,
output: &mut [Option<&'p PyAny>],
) -> PyResult<(&'p PyTuple, Option<&'p PyDict>)> {
let nargs = args.len();
let mut used_args = 0;
macro_rules! raise_error {
($s: expr $(,$arg:expr)*) => (return Err(PyTypeError::new_err(format!(
concat!("{} ", $s), fname.unwrap_or("function") $(,$arg)*
concat!("{} ", $s), fname $(,$arg)*
))))
}
let nargs = args.len();
let provided_positional_args = std::cmp::min(nargs, num_positional_params);
// Copy kwargs not to modify it
let kwargs = match kwargs {
Some(k) => Some(k.copy()?),
@ -57,8 +60,8 @@ pub fn parse_fn_args<'p>(
for (i, (p, out)) in params.iter().zip(output).enumerate() {
*out = match kwargs.and_then(|d| d.get_item(p.name)) {
Some(kwarg) => {
if i < nargs {
raise_error!("got multiple values for argument: {}", p.name)
if i < provided_positional_args {
raise_error!("got multiple values for argument '{}'", p.name)
}
kwargs.as_ref().unwrap().del_item(p.name)?;
Some(kwarg)
@ -66,15 +69,14 @@ pub fn parse_fn_args<'p>(
None => {
if p.kw_only {
if !p.is_optional {
raise_error!("missing required keyword-only argument: {}", p.name)
raise_error!("missing required keyword-only argument '{}'", p.name)
}
None
} else if i < nargs {
used_args += 1;
Some(args.get_item(i))
} else {
if !p.is_optional {
raise_error!("missing required positional argument: {}", p.name)
raise_error!("missing required positional argument '{}'", p.name)
}
None
}
@ -88,18 +90,21 @@ pub fn parse_fn_args<'p>(
raise_error!("got an unexpected keyword argument: {}", key)
}
// Raise an error when we get too many positional args
if !accept_args && used_args < nargs {
if !accept_args && num_positional_params < nargs {
raise_error!(
"takes at most {} positional argument{} ({} given)",
used_args,
if used_args == 1 { "" } else { "s" },
nargs
"takes {} positional argument{} but {} {} given",
num_positional_params,
if num_positional_params == 1 { "" } else { "s" },
nargs,
if nargs == 1 { "was" } else { "were" }
)
}
// 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);
let slice = args
.slice(num_positional_params as isize, nargs as isize)
.into_py(py);
py.checked_cast_as(slice).unwrap()
} else {
args

22
tests/test_methods.rs
View File

@ -245,6 +245,11 @@ impl MethArgs {
a + b
}
#[args(args = "*", a)]
fn get_args_and_required_keyword(&self, py: Python, args: &PyTuple, a: i32) -> PyObject {
(args, a).to_object(py)
}
#[args(a, b = 2, "*", c = 3)]
fn get_pos_arg_kw_sep1(&self, a: i32, b: i32, c: i32) -> i32 {
a + b + c
@ -365,6 +370,23 @@ fn meth_args() {
PyTypeError
);
py_run!(
py,
inst,
"assert inst.get_args_and_required_keyword(1, 2, a=3) == ((1, 2), 3)"
);
py_run!(
py,
inst,
"assert inst.get_args_and_required_keyword(a=1) == ((), 1)"
);
py_expect_exception!(
py,
inst,
"inst.get_args_and_required_keyword()",
PyTypeError
);
py_run!(py, inst, "assert inst.get_pos_arg_kw_sep1(1) == 6");
py_run!(py, inst, "assert inst.get_pos_arg_kw_sep1(1, 2) == 6");
py_run!(