classification
Title: PyArg_ParseTupleAndKeywords: support required keyword arguments
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: larry, msullivan, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2018-07-26 13:04 by serhiy.storchaka, last changed 2019-02-16 06:23 by serhiy.storchaka.

Pull Requests
URL Status Linked Edit
PR 11834 open msullivan, 2019-02-13 01:56
Messages (8)
msg322419 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-26 13:04
In Python the function can have 4 kinds of parameters:

    def f(a, b=1, *, c, d=2):

1. a is a required positional-or-keyword parameter.
2. b is an optional positional-or-keyword parameter.
3. c is a required keyword-only parameter.
4. d is an optional keyword-only parameter.

But PyArg_ParseTupleAndKeywords() supports only 3 of them. Keyword-only parameters can only be optional.

    PyArg_ParseTupleAndKeywords(args, kwds, "O|O$O", kwlist, &a, &b, &d)

This makes impossible using PyArg_ParseTupleAndKeywords() for haslib.scrypt(). Currently it parses all keyword-only arguments as optional, and checks that they are specified specially, but this does not allow to use the full power of the PyArg_Parse* API.
msg322589 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-28 20:29
The problem is that PyArg_ParseTupleAndKeywords() supports required keyword-only parameters, but only if all parameters are required.

"O|O$O" -- last two are optional.
"OO$O" -- all are required.

This makes designing a consistent syntax more difficult. '|' currently is not allowed after '$'. If allow it with the meaning that all arguments before it are required, and after it -- optional, this will allow to declare required and optional keyword-only parameters, but optional positional-or-keyword parameters could not be used in the same function.
msg322592 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-28 21:13
Support for keyword-only parameters was added in issue14328. Although the implementation never matched the documentation in the case when '|' is not specified before '$'.
msg334736 - (view) Author: Michael Sullivan (msullivan) * Date: 2019-02-02 03:54
How about adding another sigil that indicates that subsequent keyword-only arguments are required? So then your example becomes (using ` as a totally strawman option):

    PyArg_ParseTupleAndKeywords(args, kwds, "O|O$O`O", kwlist, &a, &b, &d, &c)

It's a little complicated but so is Python argument processing, so maybe that makes sense.

I can submit a PR for this.
msg335590 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-02-15 08:29
I thought about this variant. The problem is that you need to scan the format string to the end to determine whether arguments following $ are optional or not. This adds performance penalty for all existing uses of PyArg_ParseTupleAndKeywords() with $. Your PR changes the current behavior, and this is unacceptable too, because it can lead to crashes and other errors in existing code.

It is possible to add this feature without breakage and performance loss. Just allow $ to be used twice, for required and optional arguments. Then

    PyArg_ParseTupleAndKeywords(args, kwds, "O$O|O$O", kwlist, &a, &c, &b, &d)

will define four parameters in the following order: required positional-or-keyword, required keyword-only, optional positional-or-keyword, optional keyword-only.

My doubts are that this order is different from the order of parameters as defined in Python functions: required positional-or-keyword, optional positional-or-keyword, required keyword-only, optional keyword-only. I afraid this can cause confusions.

Although, the first order (all required parameters are first) is more efficient for processing. So perhaps for compatibility, performance, and the simplicity of the implementation we will accept it.
msg335593 - (view) Author: Michael Sullivan (msullivan) * Date: 2019-02-15 09:42
The point about a performance penalty is fair---my PR does add a search for the '@' (which I spelled as '`' in my example above) sigil whenever it encounters a '|'. (Though I'm not sure how big the impact would be? Format strings are small so a quick scan through it should be pretty fast. Could be measured.)

I am missing how my PR changes the current behavior, though? As far as I can tell it should behave exactly the same unless there is a '@' in the format string.
msg335666 - (view) Author: Michael Sullivan (msullivan) * Date: 2019-02-16 04:57
A downside of the "allow $ twice" approach is that it means splitting up the positional arguments, and a lot of the processing loop is built around the assumption that the index into the keyword list and the index into the argument tuple coincide (for positional arguments).

It can certainly be done, but I worry it'll be an invasive change
msg335669 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-02-16 06:23
> I am missing how my PR changes the current behavior, though?

Sorry, I misunderstood your changes to the documentation.

Other option is to introduce a special symbol that makes the following parameters required if used after (or instead of) $. Like in "O|O$@O|O" or "O|O@O|O". It does not look nice too.

I think we should try several different ways and choose the one that has simple implementation, minimal performance penalty and looks less confusing.
History
Date User Action Args
2019-02-16 06:23:00serhiy.storchakasetmessages: + msg335669
2019-02-16 04:57:24msullivansetmessages: + msg335666
2019-02-15 09:42:56msullivansetmessages: + msg335593
2019-02-15 08:29:22serhiy.storchakasetmessages: + msg335590
2019-02-13 01:56:36msullivansetkeywords: + patch
stage: patch review
pull_requests: + pull_request11865
2019-02-02 03:54:24msullivansetnosy: + msullivan
messages: + msg334736
2018-07-28 21:13:06serhiy.storchakasetnosy: + larry
messages: + msg322592
2018-07-28 20:29:44serhiy.storchakasetmessages: + msg322589
2018-07-26 13:04:16serhiy.storchakacreate