Patches item #1691070, was opened at 2007-03-29 23:45 Message generated for change (Comment added) made by nnorwitz You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1691070&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: Core (C code) Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Roger Upole (rupole) Assigned to: Nobody/Anonymous (nobody) >Summary: Speed up PyArg_ParseTupleAndKeywords() and improve error msg Initial Comment: This is a fix for [ 1283289 ] PyArg_ParseTupleAndKeywords gives misleading error message. It also yields a 10-15% decrease in cpu usage, depending on the number of arguments. ---------------------------------------------------------------------- >Comment By: Neal Norwitz (nnorwitz) Date: 2007-04-18 23:37 Message: Logged In: YES user_id=33168 Originator: NO The patch to the test code looks fine, although there are some style issues. Some lines over 80 columns and there needs to be spaces around = and other binary operators. I need to review the C code changes, but this patch looks interesting. Roger, in the future, can you make a single patch with all the files. It's easier to download and apply one patch with multiple files than 3 diff patches. ---------------------------------------------------------------------- Comment By: Roger Upole (rupole) Date: 2007-04-04 02:40 Message: Logged In: YES user_id=771074 Originator: YES File Added: test_getargs2.patch ---------------------------------------------------------------------- Comment By: Roger Upole (rupole) Date: 2007-04-03 22:39 Message: Logged In: YES user_id=771074 Originator: YES File Added: _testcapimodule.patch ---------------------------------------------------------------------- Comment By: Roger Upole (rupole) Date: 2007-04-03 22:26 Message: Logged In: YES user_id=771074 Originator: YES File Added: getargs_1.patch ---------------------------------------------------------------------- Comment By: Paul Hankin (paulhankin) Date: 2007-04-01 11:00 Message: Logged In: YES user_id=1740099 Originator: NO Patched code compiles and passes test suite, and looks cleaner than the code it replaces. It works on the cases the bug report. There should be unit tests added to Lib/test/test_getargs2; I've skimmed the tests in there and can't see any that test combinations of kw, tuple and positional arguments. The patch replaces significant chunks of arg parsing code, so I'd be more convinced of its correctness if there were some unit tests. Certainly there should be at least one test for the bug the patch fixes. The code is 99% written in the prevailing style, but: Source contains several over-long lines, and non-standard comments, ie you should use /* XXX something .. */ and not /* ??? something .. ??? */ Also there are a few minor spacing inconsistencies which would be nice to tidy up. The diff is one level too high up - it contains the directory Python2.5/ It applies cleanly to the trunk though, it just makes it slightly more difficult to apply the patch. The original bug report requests a post to python-dev describing the new error messages. I suggest this post includes all cases where the new code produces a different message than the old. I suggest to proceed: 1. Add unit tests, both for the bug-fix and for existing functionality which has been modified 2. Fix up comments and long lines 3. Check with python-dev about new error messages ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1691070&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
