Skip to content
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

Positional-only parameters do not follow PEP 570 #2799

Closed
RazerM opened this issue Dec 7, 2022 · 2 comments · Fixed by #2800
Closed

Positional-only parameters do not follow PEP 570 #2799

RazerM opened this issue Dec 7, 2022 · 2 comments · Fixed by #2800
Labels

Comments

@RazerM
Copy link

RazerM commented Dec 7, 2022

Bug Description

Amongst several benefits to positional-only arguments, PEP 570 demonstrates two which are not currently possible in PyO3:

  • the author may rename a positional-only parameter without it being a breaking change
  • it allows functions to accept any keyword argument but also accept a positional one

Steps to Reproduce

Let us first look at a pure Python function:

>>> def makedict(other=(), /, **kwargs):
...     return dict(other, **kwargs)
...
>>> makedict(other=1)
{'other': 1}

This is how the dict constructor works, for example. There is no possible naming of the positional-only argument that is visible (as an effect) to the caller.

When we try to implement this in Rust, like so:

#[pyfunction(other = "None", "/", kwargs = "**")]
fn makedict(py: Python<'_>, other: Option<&PyAny>, kwargs: Option<&PyDict>) -> PyResult<PyObject> {
    let dict = py.import("builtins")?.getattr("dict")?;
    Ok(dict.call((other.unwrap_or_else(|| PyTuple::empty(py)),), kwargs)?.into())
}

then we get an error

>>> makedict(other=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: makedict() got some positional-only arguments passed as keyword arguments: 'other'

Backtrace

No response

Your operating system and version

macOS Monterey

Your Python version (python --version)

3.10.7

Your Rust version (rustc --version)

rustc 1.64.0 (a55dd71d5 2022-09-19)

Your PyO3 version

0.17.2

How did you install python? Did you use a virtualenv?

pyenv

Additional Info

No response

@davidhewitt
Copy link
Member

davidhewitt commented Dec 8, 2022

Thanks for the report. I verified expected behaviour with a Python-only function. #2800 should fix.

@RazerM
Copy link
Author

RazerM commented Dec 8, 2022

Thanks for the quick response!

bors bot added a commit that referenced this issue Dec 14, 2022
2800: allow `**kwargs` to take arguments which conflict with positional-only parameters r=davidhewitt a=davidhewitt

Closes #2799 

I did a little bit of refactoring at the same time, sorry! The bit which changes behaviour is that in the now-factored-out `handle_kwargs` method, if the keyword argument name is found in the positional-only range then the varkeywords handler is now given a chance to accept it. (Lines 387-390 in new `extract_argument.rs`.)

Co-authored-by: David Hewitt <[email protected]>
bors bot added a commit that referenced this issue Dec 15, 2022
2800: allow `**kwargs` to take arguments which conflict with positional-only parameters r=davidhewitt a=davidhewitt

Closes #2799 

I did a little bit of refactoring at the same time, sorry! The bit which changes behaviour is that in the now-factored-out `handle_kwargs` method, if the keyword argument name is found in the positional-only range then the varkeywords handler is now given a chance to accept it. (Lines 387-390 in new `extract_argument.rs`.)

Co-authored-by: David Hewitt <[email protected]>
bors bot added a commit that referenced this issue Dec 17, 2022
2709: ci: add Python 3.12-dev jobs r=davidhewitt a=davidhewitt

Separately to #2708, time to start testing 3.12 alphas on CI. I imagine this might also take a while for the packages to come available.

2800: allow `**kwargs` to take arguments which conflict with positional-only parameters r=davidhewitt a=davidhewitt

Closes #2799 

I did a little bit of refactoring at the same time, sorry! The bit which changes behaviour is that in the now-factored-out `handle_kwargs` method, if the keyword argument name is found in the positional-only range then the varkeywords handler is now given a chance to accept it. (Lines 387-390 in new `extract_argument.rs`.)

2811: adjust vectorcall symbols for pypy r=davidhewitt a=davidhewitt

Closes #2738

Co-authored-by: David Hewitt <[email protected]>
bors bot added a commit that referenced this issue Dec 17, 2022
2800: allow `**kwargs` to take arguments which conflict with positional-only parameters r=davidhewitt a=davidhewitt

Closes #2799 

I did a little bit of refactoring at the same time, sorry! The bit which changes behaviour is that in the now-factored-out `handle_kwargs` method, if the keyword argument name is found in the positional-only range then the varkeywords handler is now given a chance to accept it. (Lines 387-390 in new `extract_argument.rs`.)

2811: adjust vectorcall symbols for pypy r=davidhewitt a=davidhewitt

Closes #2738

Co-authored-by: David Hewitt <[email protected]>
@bors bors bot closed this as completed in 239f8e6 Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants