Repository navigation
Implementation of PEP 3102 Keyword Only Argument #43917
Description
| BPO | 1549670 |
|---|---|
| Nosy | @gvanrossum |
| Files |
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:
assignee = 'https://github.com/gvanrossum'
closed_at = <Date 2008-01-06.22:29:46.443>
created_at = <Date 2006-08-31.01:06:34.000>
labels = ['interpreter-core']
title = 'Implementation of PEP 3102 Keyword Only Argument'
updated_at = <Date 2008-01-06.22:29:46.443>
user = 'https://bugs.python.org/jiwon'bugs.python.org fields:
activity = <Date 2008-01-06.22:29:46.443>
actor = 'admin'
assignee = 'gvanrossum'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2006-08-31.01:06:34.000>
creator = 'jiwon'
dependencies = []
files = ['7515']
hgrepos = []
issue_num = 1549670
keywords = ['patch']
message_count = 14.0
messages = ['51023', '51024', '51025', '51026', '51027', '51028', '51029', '51030', '51031', '51032', '51033', '51034', '51035', '51036']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'nnorwitz', 'jiwon']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1549670'
versions = ['Python 3.0']
Activity
jiwon commentedon Aug 31, 2006
This patch is implementation of PEP-3102, keyword-only
parameter.
Important changes include
number of keyword only argument - this is analogous to
co_argcount.
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.
Api. PyCode_New now takes number of keyword only
arguments, and new.code also takes number of keyword
only arguments.
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.
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.
nnorwitz commentedon Aug 31, 2006
Logged In: YES
user_id=33168
import.c: comment has double word 'added'
ast.c:
to return -1 after setting the error?
other changes. this makes it hard to see the real changes.
compile.c:
more than 255 positional args and kw_default_count since
there is |= kw_default_count << 8 (2 places)
surrounding code.
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.
jiwon commentedon Aug 31, 2006
Logged In: YES
user_id=595483
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.
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
Yeah, I copied it from test_with.py, but now I'm passing a
filename.
jiwon commentedon Aug 31, 2006
Logged In: YES
user_id=595483
Maybe I should mention that I've uploaded new patch that
fixed what Neal mentioned.
jiwon commentedon Sep 7, 2006
Logged In: YES
user_id=595483
The original patch crashes when a function has keyword only
arguments but no default values for those arguments, and
calls the function without enough arguments. Fixed the bug,
and added testcases for that.
gvanrossum commentedon Sep 11, 2006
Logged In: YES
user_id=6380
General comment: I think you should update or add a comment
explaining the contents of the code object as it has changed
(unless that's all described in the PEP).
There are a number of places where the code is wider than 79
characters or where the indentatiln style seems to be off.
You may have to set your tabstops to 8 spaces to see these.
E.g.:
ceval.c:
ast.c: @@ -591,6 +591,63 @@ please follow the surrounding
4-space indents; don't use tabs here
marshal.c: line 871 too long
codeobject.c: several lines too long
funcobject.c: too long at "non-dict keyword only default args");
Lib/compiler/*.py: several lines too long
Lib/test/test_new.py
Modules/pyexpat.c: @@ -279,6 +279,7 @@ indentation error
Modules/parsermodule.c: several lines too long; the loop at
"while (res && i+1 < nch)" is indented one tabstop too many
(you seem to have edited this file with ts=4?)
jiwon commentedon Sep 13, 2006
Logged In: YES
user_id=595483
The codeobject has a kwonlyargcount meaning the number of
keyword only arguments. Thus, the api for creating new code
object is changed: PyCode_New now takes kwonlyargcount.
So, the signature changes from this
PyCode_New(argcount, nlocals, stacksize, ...)
to following.
PyCode_New(argcount, kwonlyargcount, nlocals, stacksize, ...)
From python, you can access this with name
"co_kwonlyargcount" such as, co.co_kwonlyargcount just like
you access argcount with co.co_argcount.
jiwon commentedon Sep 14, 2006
Logged In: YES
user_id=595483
I made a big mistake with the grammar - currently, the
grammar doesn't allow keyword only argument after *vararg.
At some point of the development process, I just assumed
as such. Underlying logic does not assume anything about
that, so I'll try to fix it as soon as possible
gvanrossum commentedon Sep 15, 2006
Logged In: YES
user_id=6380
I see. The Grammar should simply make the NAME after the '*'
optional. Shouldn't be too hard to fix should it?
jiwon commentedon Oct 9, 2006
Logged In: YES
user_id=595483
The new patch that I am uploading fixes the Grammar
problem - originally I made mistake not allowing *vararg
after keyword only argument. The python compiler package
doesn't work now, but everything else should work now.
gvanrossum commentedon Oct 27, 2006
Logged In: YES
user_id=6380
Checked in as r52491.
I'm keeping this open awaiting a patch for the compiler package.
jiwon commentedon Nov 22, 2006
Now the compiler package works. The patch passes regrtest with "-u all" option and "-u compiler" option (except test_frozen.py). The patch is against latest p3yk branch HEAD. The last patch accidentally included some test code in test_compiler.py (and which is committed to the svn) so this patch includes fix for that.
Ah, Guido, when you committed the patch (previous version) last time, you did not add Lib/test/test_keywordonlyarg.py so, this time, don't forget to add it :) Also, last time when we talked, I think you said that the patch makes some test case fail on your laptop machine. You might want to check that.
gvanrossum commentedon Nov 22, 2006
Submitted.
Jiwon, can you mail me another copy of test_keywordinoyarg.py? I believe I lost it.
gvanrossum commentedon Nov 23, 2006
All submitted. (test_keywordonlyarg.py too)