[Patches] [ python-Patches-1549670 ] Implementation of PEP 3102 Keyword Only Argument

[SourceForge.net]

Patches item #1549670, was opened at 2006-08-31 10:06
Message generated for change (Comment added) made by jiwon
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1549670&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Parser/Compiler
Group: Python 3000
Status: Open
Resolution: None
Priority: 5
Submitted By: Jiwon Seo (jiwon)
Assigned to: Nobody/Anonymous (nobody)
Summary: Implementation of PEP 3102 Keyword Only Argument

Initial Comment:
This patch is implementation of pep 3102, keyword-only
parameter.

Important changes include

 * code object now has co_kwonlyargcount to keep the
number of keyword only argument - this is analogous to
co_argcount.
 * function object now has func_kwdefaults (dictionary)
to keep the mapping between keyword only arguments and
defaults for them. Only kwonly argument with default
values are in the dictionary - this is analogous to
func_defaults.
 * APIs for code object changed - both C API and Python
Api. PyCode_New now takes number of keyword only
arguments, and new.code also takes number of keyword
only arguments.
 * MAKE_FUNCTION now takes another oparg, which is
number of default keyword only arguments - and the name
of keyword only argument and its default value are in
the value stack - it is similar to oparg of CALL_FUNCTION.
 * MAGIC in import.c changed, since bytecode is changed.

That's pretty much everything that's important, and the
rest is in the code itself.

And my patch passes all regression tests excepts
test_frozen.py, which depends on the hard-coded mashal
value, which needs to be regenerated when bytecode
changes. However, freeze.py is broken - specifically,
modulefinder.py is broken as Guido said. So, currently,
I commented it out.

----------------------------------------------------------------------

>Comment By: Jiwon Seo (jiwon)
Date: 2006-09-01 06:03

Message:
Logged In: YES 
user_id=595483

>>  * it looks like a bunch of lines had whitespace, but no
>> other changes.  this makes it hard to see the real changes.

I changed some tabs in ast_for_argument function to spaces -
tabs and spaces are mixed, and it's hard to edit. It would
have been nice if I separate that white space conversion
into another patch, but since it's p3yk branch I thought it
can be allowed.


>> when you say it passes all tests, did you run with -u all?
>> the compiler doesn't do all the tests without it (and one
>> or
>> two other tests too).

Yup, it passes all the tests except test_frozen and
test_linuxaudiodev.py(maybe because my box doesn't have
audio device..?) and some skipped tests including
test_bsddb3.py, etc


>> in the new test, i think you need to pass a name for the
>> filename param to compile.  I think there was some platform
>> (windows?) that crapped out with an empty filename.  (if you
>> copied from an existing test, then i must be remembering
>> something different.)

Yeah, I copied it from test_with.py, but now I'm passing a
filename.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2006-08-31 14:42

Message:
Logged In: YES 
user_id=33168

import.c: comment has double word 'added'
ast.c: 
 * why is static commented out for ast_for_arguments?
 * in handle_keywordonly_args, doesn't the default case need
to return -1 after setting the error?
 * need to change the // comments to /* */
 * it looks like a bunch of lines had whitespace, but no
other changes.  this makes it hard to see the real changes.

compile.c:
 * i think there is a bug in the arglength calc if there are
more than 255 positional args and kw_default_count since
there is |= kw_default_count << 8 (2 places)
 * return should go on its own line like the rest of the
surrounding code.
 * why kwonlyargs and kw_args?  the underscore seems
inconsistent

in the compiler package, I didn't see a change to
MAKE_FUNCTION similar to the one in compile.c for the opcode
stack effect.

the change to regrtest.py doesn't seem necessary (just an
added blank line)

there should be a lot more tests added for both positive and
negative conditions (syntax errors).  Include variants with
func(**kwargs) func(*args), etc in calls.

it's not good to comment out the tests in test_frozen. 
otherwise, we will probably forget about the problems.

when you say it passes all tests, did you run with -u all?
the compiler doesn't do all the tests without it (and one or
two other tests too).

in the new test, i think you need to pass a name for the
filename param to compile.  I think there was some platform
(windows?) that crapped out with an empty filename.  (if you
copied from an existing test, then i must be remembering
something different.)

In testRaiseErrorWhenFuncall(), you can use
self.assertRaises, at least for most of the cases.

you need a test_main for this test to be run from regrtest.



----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1549670&group_id=5470