Repository navigation
Argument Clinic doesn't support named constants as default values #64343
Description
Activity
For example it doesn't support PY_SSIZE_T_MAX as default value for Py_ssize_t argument. Is there any way to specify it? This is required for converting the _sre module.
Here's the problem. Let's say I gave you a way of specifying a symbolic constant for the default value for C. What value should we use for the default value in Python? Keep in mind, it has to be expressed as a static value that can be stored as a string as part of the __text_signature__ at the front of the docstring.
If you can answer that question, we can solve the problem.
As an example, consider pattern_match() in _sre.c. This implements the match method on a pattern object; in other words, re.compile().match(). The third parameter, endpos, defaults to PY_SSIZE_T_MAX in C. What should inspect.Signature() report as the default value for endpos? We can't look up the proper value for PY_SSIZE_T_MAX, because the default value inspect.Signature() uses was written out as text when Argument Clinic processed the function.
As a hint, the documentation for match() dodges this problem by outright lying. It claims that the prototype for the function is:
match(string[, pos[, endpos]])
which is a lie. pattern_match() parses its arguments by calling PyArg_ParseTupleAndKeywords() with a format string of "O|nn". which means, for example, you could call:
match("abc", endpos=5)
The documentation claims this is invalid but it works fine.
In other words, it's a sticky problem, and the people who had it before us didn't try to solve it properly.
Not always default value can be expressed in Python. Current docstring for the match() method says "match(string[, pos[, endpos]])". For this particular case it can be written as "match(string, pos=0, endpos=sys.maxsize)". And this is very common case (see also str.find(sub[, start[, end]]), etc). Perhaps there are cases in which the default value is not PY_SSIZE_T_MAX and None is not accepted (as for dir_fd parameter in os functions).
So as minimum Argument Clinic can support sys.maxsize, and as maximum, it should support arbitrary values.
"sys.maxsize" won't work. inspect.Signature parses the __text_signature__ using ast.parse, and it only recognizes constant values and named constants (True/False/None) for the default value for parameters. Playing with ast, it looks like we'd have to support a tree of Attribute nodes, then look up sys (or whatever) and recursively getattr on it. Which would be new code.
Anyway it seems like a bad idea:
import inspect import re import sys sys.maxsize = 3 inspect.signature(re.compile(".*").match)
Then perhaps these methods can not be converted to use Argument Clinic now.
Attached is an example of how we could support simple named constants in __text_signature__. The change to posixmodule.c is just a hack to exercise the code in inspect.Signature; I didn't have a good example handy.
When I apply the patch and run "x.py", I see:
(path, *, dir_fd=None, follow_symlinks=True, fake=9223372036854775807)This patch is just an example; I don't propose to check it in in this state. (We'd still need to modify Argument Clinic to support these named constants, too.)
Whoops, forgot to attach the file. here it is.
It seems a little unfortunate that you lose the symbolic default, especially since the expanded number just appears random.
>>> print(inspect.signature(os.stat)) (path, *, dir_fd=None, follow_symlinks=True, fake=9223372036854775807) >>> sys.maxsize 9223372036854775807It is unfortunate, but on the other hand that's what happens in Python too:
>>> import sys >>> import inspect >>> def foo(blah=sys.maxsize): pass ... >>> str(inspect.signature(foo)) '(blah=9223372036854775807)'Nick proposed something we could use to fix these (I think he called them "named constants") but it's not in the language yet. Anyway propagating those all the way from Argument Clinic to inspect.Signature would be tricky.
Could you live with this being checked in to 3.4?
On Jan 06, 2014, at 08:23 PM, Larry Hastings wrote:
Could you live with this being checked in to 3.4?
For sure!
Attached is a patch supporting simple symbolic constants. It works from beginning to end--you specify it in the Argument Clinic input and it shows up in the inspect.Signature and uses the constant in the generated C code.
One complication: when using one of these constants, I force you to specify a c_default. Which is now automatically supported on all converters, as is py_default.
I converted pattern_match in _sre.c as a sample. I won't check that part of the patch in; it's just to give you an idea of what it looks like for the user.
I proposed contrary approach. Allow docstring to fake signature.
>>> import sys >>> import inspect >>> def foo(blah=sys.maxsize): "foo(blah=sys.maxsize)" pass ... >>> str(inspect.signature(foo)) '(blah=sys.maxsize)'storchaka: You propose a result, not an approach. How do you propose we do that?
In any case, I think making *that* work would be way too big a change for 3.4. Whatever you proposed would only be appropriate for 3.5.
Here's a second patch; I think this is ready to go in
I cleaned up the node parsing a lot. It now knows how to parse the following types of default values:
- Number (this applies to both ints and floats)
- String ('hello')
- Attribute (sys.maxsize)
- Named constant (None, True, False)
As long as I'm fixing this, I want to add support for any other values we're going to want. Did I miss any? Note that we can't really support container types, because initializing them on the C side isn't practical.
Also, the tests don't exercise all of the above possible node types, just because I couldn't remember builtins that used those types. We'll fill in the remaining tests as we port new functions to Argument Clinic.
If I can get a quick LGTM review, I'll check this in.
Oops! I forgot to actually attach the new patch. I'm dumb.
At Antoine's suggestion, I added a custom function to testcapi that exercises all the different possible types for default values in a text signature. Also the docs have been updated. LGTU?
Incorporated suggestions from Serhiy. Thanks, Serhiy!
python-dev commented
on Jan 7, 2014 python-devmannequinMannequinNew changeset c96dba33f019 by Larry Hastings in branch 'default':
Issue bpo-20144: Argument Clinic now supports simple constants as parameter
http://hg.python.org/cpython/rev/c96dba33f019Argument Clinic now supports simple constants like "sys.maxsize" as default values for arguments for builtins. I'm assuming this gets you basically what you wanted; if this isn't sufficient please open a new issue.
Thank you Larry for all your fixes.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: