This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Optional Argument Syntax
Type: Stage:
Components: Interpreter Core Versions: Python 3.0
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: gvanrossum Nosy List: gvanrossum, jimjjewett, nnorwitz, tonylownds
Priority: normal Keywords: patch

Created on 2006-12-02 20:53 by tonylownds, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
opt_arg_ann.patch tonylownds, 2006-12-28 01:28
peepholer_and_max_annotations.patch tonylownds, 2007-01-06 20:05
pydoc.patch tonylownds, 2007-01-06 21:03
make_closure_fix.patch tonylownds, 2007-01-14 20:32
pydoc.patch tonylownds, 2007-02-28 03:23
Messages (30)
msg51437 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-02 20:53
This patch implements optional argument syntax for Python 3000. The patch still has issues; I am posting so that Collin Winters can add a link to the PEP.

The syntax implemented is roughly:

def f(arg:expr, (nested1:expr, nested2:expr)) -> expr:
  suite

The function object has a new attribute, func_annotations that maps from argument names to the result of the expression. The return annotation is stored with a key of 'return'.

Lambda's syntax doesn't support annotations.

This patch alters the MAKE_FUNCTION opcode. I have an implementation that built the func_annotations dictionary in bytecode as well but it was bigger and slower.
msg51438 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-04 01:24
This patch implements optional argument syntax for Python 3000. The patch still has issues:
1. test_ast and test_scope fail.
2. Running the test suite after compiling the library with the compiler package causes failures
3. no docs
4. C-code reference counts and error checking needs a review

The syntax implemented is roughly:

def f(arg:expr, (nested1:expr, nested2:expr)) -> expr:
suite

The function object has a new attribute, func_annotations that maps from argument names to the result of the expression. The return annotation is stored with a key of 'return'.

Lambda's syntax doesn't support annotations.

The ast format has changed for the builtin compiler and the compiler package. A new token was added, '->' (called RARROW in token.h). token.py lost ERRORTOKEN after re-generating, I don't know why. I added it back manually.

msg51439 - (view) Author: Jim Jewett (jimjjewett) Date: 2006-12-11 17:29
Could you rename it to "argument annotations"?  "optional argument" makes me think of the current keyword arguments, that can be but don't have to be passed.

-jJ
msg51440 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2006-12-20 01:22
Applying the patch fails, probably due to recent merge activities in the p3yk branch. Can I inconvenience you with a request to regenerate the patch from the branch head?
msg51441 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-12-20 08:38
When regenerating the patch, can you also remove non-functional changes such as removing unneeded parens and whitespace changes.  Also, please try to keep the same formatting in the file wrt tabs and spaces and don't move code around.  I know this is a pain and inconsistent.  I think I changed ast.c to be all 4 space indents with spaces only.

In compiler_simple_arg(), don't you need to check if annotation is NULL when returned from ast_for_expr?  Otherwise an undetected error would go through, wouldn't it?

In compiler_complex_args(), don't you need to set the ast_error (or a SystemError) if the switch isn't a tname, vname, or LPAR?  I don't like the names tname and vname.  Also they seem inconsistent.  Aren't all the other names all CAPS?

In hunk, @@ -602,51 +625,75 @@ remove the commented out code.  We shouldn't use any // style comments either.
Can you improve the error msg for kwdefaults == NULL?  (Thanks for adding it!)
Check annotation for NULL if returned from ast_for_expr?

BTW, the AST code in this area was tricky code which had some bugs.  Did you test with adding extra parentheses and singleton tuples?

I'm not sure if Guido preferred syntax -> vs a keyword 'as' for the return type.

In symtable.c remove the printfs.  They should probably be SystemErrors or something.

I would definitely prefer the annotations baked into the code object so there are no changes to ceval.

Did we decide if lambda was going to require parens around the arguments?  If so, it could support annotations, right?  (No comment on the usefulness of annotations for lambdas. :-)

In compiler_visit_argannotation, you should return the result from PyList_Append and can remove the comment about checking for errors.  Also, I believe the INCREF is not needed, it will be done by PyList_Append.
Same deal with returning result of compiler_visit_argannotations() (the one with an s).

Need to check for PyList_New() returning NULL in compiler_visit_annotations().
Lots more error checking needs to be added in this area.

Dammit, I really want to use Mondrian for these comments!  (Sorry Tony, not your fault, I'm just having some bad memories at this point cause I have to keep providing the references.)

This patch looks very complete in that it updates things like the compiler package and the parsermodule.c.  Good job!  This is a great start.
msg51442 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-12-20 09:25
Nix this comment:  I would definitely prefer the annotations baked into the code object so
there are no changes to ceval.  I see that Guido wants it the way it currently is which makes sense for nested functions.  There should probably be a test with nested functions even though it really shouldn't be different.  The test will verify that.
msg51443 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-20 18:04
I'll work on code formatting and the error checking and other cleanup. Open to other names than tname and
vname, I created those non-terminals in order to use the same code for processing "def" and "lambda". Terminals 
are caps IIUC. 

I did add a test for the multi-paren situation. 2.5 had that bug too.

Re: no changes to ceval, I tried generating the func_annotations dictionary using 
bytecodes. That doesn't change the ceval loop but was more code and was slower. 
So there is a way to avoid ceval changes.

Re: deciding if lambda was going to require parens around the arguments,
I don't think there was any decision, and yes annotations would be easily supportable.
Happy to change if there is support, it's backwards incompatible.

Re: return type syntax, I have only seen the -> syntax (vs a keyword 'as') on Guido's blog.

Thanks for the comments!
msg51444 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-20 22:13
Changes:
1. Updated to apply cleanly
2. Fix to compile.c so that test_complex_args passes

Open implementation issues:
1. Neal's comments
2. test_scope fails
3. Output from Lib/compiler does not pass test_complex_args


File Added: opt_arg_ann.patch
msg51445 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-21 20:21
Changes:
1. Address Neal's comments (I hope)
2. test_scope passes
3. Added some additional tests to test_compiler

Open implementation issues:
1. Output from Lib/compiler does not pass test_complex_args, test_scope, possibly more.

File Added: opt_arg_ann.patch
msg51446 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-22 20:15
Changes:
1. Fix crasher in Python/symtable.c -- annotations were visited inside the function scope
2. Fix Lib/compiler issues with Lib/test/test_complex_args. 

Output from Lib/compiler does not pass all tests, same failures as in HEAD of p3yk branch.

File Added: opt_arg_ann.patch
msg51447 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2006-12-22 21:41
Thanks for the progress!  There are still a few lines ending in whitespace or lines that are longer than 80 chars (and weren't before).  Mind cleaning those up?

Also ceval.c:2305 and compile.c:1440 contain code that gcc 2.95 won't compile (the 'int' declarations ought to be moved to the start of the containing {...} block); I think this style is not C89 compatible.
msg51448 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-23 15:53
Fixed the non-C89 style lines and the formatting (hopefully in compatible style :)
File Added: opt_arg_ann.patch
msg51449 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-23 19:05
Initial patch to implement keyword-only arguments and annotations support for pydoc and inspect.
Tests do not exercise these features, yet.

Output for annotations that are types is special cased so that for:

def intmin(*a: int) -> int: pass

...help(intmin) will display:

intmin(*a: int) -> int

File Added: pydoc.patch
msg51450 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2006-12-28 00:40
I believe I've found a leak in the code that adds annotations to a function object. See this session:

>>> x = object()
>>> import sys
>>> sys.getrefcount(x)
2
>>> for i in range(100):
...  def f(x: x): pass
...
>>> del f
>>> sys.getrefcount(x)
102
>>>

At first I thought this could be due to the code added to the MAKE_FUNCTION opcode, but I don't see a leak there. More likely func_annotations is not being freed when a function object is deleted.
msg51451 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-28 01:28
Fixed in latest patch. Also added VISIT call for func_annotations.
File Added: opt_arg_ann.patch
msg51452 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2006-12-28 01:38
Thanks!  Is there anything else that you think needs to be done before I check this in?  The core code looks alright to me; I can't be bothered with reviewing the ast stuff or the compiler package since I don't know enough about these, but given that it compiles things correctly I'm not so worried about those.

What's the status of the pydoc patch? Are you still working on that?
msg51453 - (view) Author: Tony Lownds (tonylownds) Date: 2006-12-28 03:04
Nothing else on the C side of things. The pydoc patch works well for me; more tests ought to be added for function annotations and also for keyword-only arguments, but perhaps that can be added on as a later patch after checkin.
msg51454 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-12-28 06:53
I'm skipping the pydoc patch.  Didn't even look at it.  I don't have the refleak, but I changed some calls and may have fixed it.

Committed revision 53170.

Leaving open to deal with the pydoc patch.
msg51455 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-01-04 04:30
I'm not sure it's right to just change the signature of the various functions in inspect.py; that would break all existing code using that module (and there definitely are other users besides pydoc).  It would be better to add new methods that provide access to the additional functionality.  Or do you think that everyone will have to change their code anyway?
msg51456 - (view) Author: Tony Lownds (tonylownds) Date: 2007-01-04 05:17
I think everyone should update have to update their uses of getargspec and friends, because otherwise they will silently mis-handle keyword-only arguments.
msg51457 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-01-04 05:22
Well, it depends on the context whether that matters.  The kw-only args could just be included in the positional args (which have names anyway) and that wouldn't be so bad for some apps.
msg51458 - (view) Author: Tony Lownds (tonylownds) Date: 2007-01-04 07:12
For getargs and getargvalues, including the names in positional args is an excellent strategy.
There are uses (in cgitb) in the stdlib for getargvalues that then wouldn't need to be changed. 

The 2 uses of getargspec in the stdlib (one of which I missed, in DocXMLRPCServer) are both 
closely followed by formatargspec. I think those APIs should change or information will be lost. 

Alternatively, a new function (hopefully with a better name than getfullargspec :) could be 
made and getargspec could retain its API, but raise an error when keyword-only arguments are 
present.

def getargspec(func):
  args, varargs, kwonlyargs, kwdefaults, varkw, defaults, ann = getfullargspec()
  if kwonlyargs:
     raise ValueError, "function has keyword-only arguments, use getfullargspec!"
  return args, varargs, varkw, defaults

I'll update the patch to fix getargvalues and DocXMLRPCServer this weekend.
msg51459 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-01-04 17:53
I like the following approach: (1) the old API continues to work for all functions, but provides incomplete information (not losing the kw-only args completely, but losing the fact that they are kw-only); (2) add a new API that provides all the relevant information.

Maybe the new API should not return a 7-tuple but rather a structure with named attributes; that makes it more future-proof.

Sorry, I don't have any good suggestions for new names.
msg51460 - (view) Author: Tony Lownds (tonylownds) Date: 2007-01-06 20:05
Change peepholer to not bail in the presence of EXTENDED_ARG + MAKE_FUNCTION.
Enforce the natural 16-bit limit of annotations in compile.c.

File Added: peepholer_and_max_annotations.patch
msg51461 - (view) Author: Tony Lownds (tonylownds) Date: 2007-01-06 21:03
I tried to implement getargspec() as described, and unfortunately there 
is another wrinkle to consider. Keyword-only arguments may or may not 
have defaults. So the invariant described in getargspec()'s docstring can't 
be maintained when simply appending keyword-only arguments.

    A tuple of four things is returned: (args, varargs, varkw, defaults).
    'args' is a list of the argument names (it may contain nested lists).
    'args' will include keyword-only argument names. 
    'varargs' and 'varkw' are the names of the * and ** arguments or None.
    'defaults' is an n-tuple of the default values of the last n arguments.

The attached patch adds an 'getfullargspec' API that returns complete 
information; 'getargspec' raises an error if information would be lost; the order 
of arguments in 'formatargspec' is backwards compatible, so that
formatargspec(*getargspec(f)) == formatargspec(*getfullargspec(f)) when
getargspec(f) does not raise an error.

PEP 362 could and probably should replace the new getfullargspec() function,
so I did not implement an API more complicated than a tuple.

File Added: pydoc.patch
msg51462 - (view) Author: Tony Lownds (tonylownds) Date: 2007-01-14 20:31
Combines the code paths for MAKE_FUNCTION and MAKE_CLOSURE. 
Fixes a crash where functions with closures and either annotations or 
keyword-only arguments result in MAKE_CLOSURE, but only 
MAKE_FUNCTION has the code to handle annotations or keyword-only
arguments.

Includes enough tests to trigger the bug.
msg51463 - (view) Author: Tony Lownds (tonylownds) Date: 2007-01-14 20:32
File Added: make_closure_fix.patch
msg51464 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-02-26 20:58
Tony, I am checking make_closure_fix.patch and peepholer_and_max_annotations.patch.

Unfortunately, something happened to pydoc.py so that pydoc.patch no longer applies and I don't want to have to think about how to fix it.  Can you have a look at this and submit a revised patch?
msg51465 - (view) Author: Tony Lownds (tonylownds) Date: 2007-02-28 03:23
File Added: pydoc.patch
msg51466 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-02-28 22:05
Thanks!  Committed as r54043.  If you ever feel the urge to add unittests for the new functions, just mail them to me, as I'm closing this patch. ;-)
History
Date User Action Args
2022-04-11 14:56:21adminsetgithub: 44297
2008-01-06 22:29:46adminsetkeywords: - py3k
versions: + Python 3.0
2006-12-02 20:53:33tonylowndscreate