Derby #15: Convert 50 sites to Argument Clinic across 9 files #64351

brettcannon · 2014-01-06T20:18:13Z

BPO	20152
Nosy	@loewis, @brettcannon, @larryhastings, @serhiy-storchaka
Files	import_ac.diff grp_derby.diff: grp module converted to Argument Clinic spwd_derby.diff: spwd converted to Argument Clinic pwd_derby.diff: pwd converted to Argument Clinic fcntl_derby.diff: fcntl converted to Argument Clinic (except for ioctl) pyexpat_derby.diff: pyexpat converted, relying on default self converter multibytecodec_derby.diff: multibytecodec with default self converter array_derby.diff: array module converted to AC cmath_derby.diff: all of cmath converted fcntl_derby.diff

^{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/brettcannon'
closed_at = <Date 2014-11-10.01:26:34.392>
created_at = <Date 2014-01-06.20:18:13.317>
labels = ['extension-modules', 'type-feature']
title = 'Derby #15: Convert 50 sites to Argument Clinic across 9 files'
updated_at = <Date 2014-11-10.01:26:34.391>
user = 'https://github.com/brettcannon'

bugs.python.org fields:

activity = <Date 2014-11-10.01:26:34.391>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2014-11-10.01:26:34.392>
closer = 'brett.cannon'
components = ['Extension Modules']
creation = <Date 2014-01-06.20:18:13.317>
creator = 'brett.cannon'
dependencies = []
files = ['33351', '33511', '33513', '33515', '33591', '33828', '33829', '33831', '33839', '37144']
hgrepos = []
issue_num = 20152
keywords = ['patch']
message_count = 35.0
messages = ['207474', '207598', '207625', '207641', '207808', '207809', '208347', '208351', '208353', '208357', '208653', '208668', '209788', '209789', '209826', '224141', '224142', '225678', '225679', '225680', '225681', '225691', '225692', '225694', '225695', '229035', '229036', '229362', '230819', '230822', '230857', '230858', '230859', '230927', '230930']
nosy_count = 5.0
nosy_names = ['loewis', 'brett.cannon', 'larry', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20152'
versions = ['Python 3.5']

brettcannon · 2014-01-06T20:18:13Z

Since _imp isn't directly exposed it isn't a priority to do the conversion, but for maintenance and such it should still occur.

larryhastings · 2014-01-07T20:57:11Z

I'm normalizing the names of these issues so they're easier to read and to search for.

brettcannon · 2014-01-07T23:55:49Z

I have attached my conversion of import.c. Larry, can you look it over to make sure I did it as expected? I tried to not have to write my own converter for O& as the docs seem to suggest object(converter='...') should work, but I got an error instead.

larryhastings · 2014-01-08T00:21:06Z

(Brett: in order to avoid creating 129 differet issues on the tracker, I'm bundling files together into groups of 50 sites. You don't have to tackle the whole list if you don't want to.)

This issue is part of the Great Argument Clinic Conversion Derby,
where we're trying to convert as much of Python 3.4 to use
Argument Clinic as we can before Release Candidate 1 on January 19.

This issue asks you to change the following bundle of files:
Python/import.c: 8 sites
Modules/cmathmodule.c: 8 sites
Modules/cjkcodecs/multibytecodec.c: 8 sites
Modules/arraymodule.c: 8 sites
Modules/pyexpat.c: 7 sites
Modules/fcntlmodule.c: 7 sites
Modules/pwdmodule.c: 2 sites
Modules/spwdmodule.c: 1 sites
Modules/grpmodule.c: 1 sites

Talk to me (larry) if you only want to attack part of a bundle.

For instructions on how to convert a function to work with Argument
Clinic, read the "howto":
http://docs.python.org/dev/howto/clinic.html

python-dev · 2014-01-10T00:03:35Z

New changeset 1f3242fb0c9c by Brett Cannon in branch 'default':
Issue bpo-20152: import.c now uses Argument Clinic.
http://hg.python.org/cpython/rev/1f3242fb0c9c

brettcannon · 2014-01-10T00:04:11Z

Now it's:

[Modules/cmathmodule.c](https://github.com/python/cpython/blob/main/Modules/cmathmodule.c): 8 sites
[Modules/cjkcodecs/multibytecodec.c](https://github.com/python/cpython/blob/main/Modules/cjkcodecs/multibytecodec.c): 8 sites
[Modules/arraymodule.c](https://github.com/python/cpython/blob/main/Modules/arraymodule.c): 8 sites
[Modules/pyexpat.c](https://github.com/python/cpython/blob/main/Modules/pyexpat.c): 7 sites
[Modules/fcntlmodule.c](https://github.com/python/cpython/blob/main/Modules/fcntlmodule.c): 7 sites
[Modules/pwdmodule.c](https://github.com/python/cpython/blob/main/Modules/pwdmodule.c): 2 sites
[Modules/spwdmodule.c](https://github.com/python/cpython/blob/main/Modules/spwdmodule.c): 1 sites
[Modules/grpmodule.c](https://github.com/python/cpython/blob/main/Modules/grpmodule.c): 1 sites

brettcannon · 2014-01-17T17:09:16Z

I'm going to start tackling some of these today and checking them in directly. If I don't finish I will update the bug with what I didn't get to.

brettcannon · 2014-01-17T18:38:26Z

Patch for multibytecodec.c.

Not sure how you calculated locations, Larry, but this file had 11 things to change, so some other work divisions might be off.

brettcannon · 2014-01-17T18:55:22Z

After finding out that the initial site counts were off I went through and manually counted each file in this Derby group to get a more accurate idea of how much work each file would be.

[Modules/cmathmodule.c](https://github.com/python/cpython/blob/main/Modules/cmathmodule.c): 8 sites
[Modules/arraymodule.c](https://github.com/python/cpython/blob/main/Modules/arraymodule.c): 27 sites
[Modules/pyexpat.c](https://github.com/python/cpython/blob/main/Modules/pyexpat.c): 11 sites
[Modules/fcntlmodule.c](https://github.com/python/cpython/blob/main/Modules/fcntlmodule.c): 4 sites
[Modules/pwdmodule.c](https://github.com/python/cpython/blob/main/Modules/pwdmodule.c): 3 sites
[Modules/spwdmodule.c](https://github.com/python/cpython/blob/main/Modules/spwdmodule.c): 2 sites
[Modules/grpmodule.c](https://github.com/python/cpython/blob/main/Modules/grpmodule.c): 3 sites

brettcannon · 2014-01-17T19:36:15Z

Might pick this up later today (or never), but taking a break for now.

Larry, I have patches for grp, spwd, _multibytecodec, and pwd I'd like you to give a once-over to make sure I didn't miss anything.

The only one that I think has an obvious improvement is in pwd where there is custom logic to deal with a PyArg_ParseTuple() failure. Does Argument Clinic have a way to specify custom logic in that instance?

With these patches the TODO for this derby group is:

[Modules/cmathmodule.c](https://github.com/python/cpython/blob/main/Modules/cmathmodule.c): 8 sites
[Modules/arraymodule.c](https://github.com/python/cpython/blob/main/Modules/arraymodule.c): 27 sites
[Modules/pyexpat.c](https://github.com/python/cpython/blob/main/Modules/pyexpat.c): 11 sites
[Modules/fcntlmodule.c](https://github.com/python/cpython/blob/main/Modules/fcntlmodule.c): 4 sites

cmath might not be worth doing, though, for 3.4 as it already has custom macros to create function definitions.

brettcannon · 2014-01-21T15:19:54Z

Here is fcntl converted. I didn't do fcntl.ioctl as it has a crazy set of possible signatures which I simply didn't want to deal with.

brettcannon · 2014-01-21T16:36:22Z

Here is pyexpat converted. Couldn't do ErrorString as the 'l' format isn't supported; issue bpo-20332.

brettcannon · 2014-01-31T13:43:38Z

Here is pyexpat updated with ErrorString converted (and generally updated to manage changes now required by AC for class definitions).

brettcannon · 2014-01-31T13:44:44Z

Should only have two modules left to convert:

[Modules/cmathmodule.c](https://github.com/python/cpython/blob/main/Modules/cmathmodule.c): 8 sites
[Modules/arraymodule.c](https://github.com/python/cpython/blob/main/Modules/arraymodule.c): 27 sites

I have already started arraymodule.c; it's just taking a while.

brettcannon · 2014-01-31T20:20:55Z

It took a bit of finessing but I managed to convert cmath in a way that didn't make it worse compared to before AC.

I now consider this part of the derby done and ready for Larry to review.

loewis · 2014-07-27T16:10:02Z

multibytecodec_derby.diff looks fine, please apply.

loewis · 2014-07-27T16:21:10Z

The cmath patch fails to apply; please update it.

python-dev · 2014-08-22T15:45:11Z

New changeset c2d71f9afa0b by Brett Cannon in branch 'default':
Issue bpo-20152: Convert _multibytecodecs to Argument Clinic.
http://hg.python.org/cpython/rev/c2d71f9afa0b

brettcannon · 2014-08-22T15:47:05Z

To make it easier to track what is left to commit:

grp
spwd
fcntl
pyexpat
array
cmath [Martin says this no longer applies cleanly]

I should also mention all of these patches probably need to shift to output preset file as well.

python-dev · 2014-08-22T15:52:52Z

New changeset 2617db7e43a3 by Brett Cannon in branch 'default':
Issue bpo-20152: Convert the grp module to Argument Clinic.
http://hg.python.org/cpython/rev/2617db7e43a3

brettcannon · 2014-08-22T15:53:20Z

To make it easier to track what is left to commit:

spwd
fcntl
pyexpat
array
cmath [Martin says this no longer applies cleanly]

python-dev · 2014-08-22T17:59:30Z

New changeset b3aa30f474c4 by Brett Cannon in branch 'default':
Issue bpo-20152: Port the spwd module to Argument Clinic.
http://hg.python.org/cpython/rev/b3aa30f474c4

python-dev · 2014-08-22T18:03:58Z

New changeset c48980af7df2 by Brett Cannon in branch 'default':
Issue bpo-20152: Port the pwd module to Argument Clinic.
http://hg.python.org/cpython/rev/c48980af7df2

python-dev · 2014-08-22T18:23:27Z

New changeset 0c2792a8f9db by Brett Cannon in branch 'default':
Issue bpo-20152: Port pyexpat to Argument Clinic.
http://hg.python.org/cpython/rev/0c2792a8f9db

brettcannon · 2014-08-22T18:24:32Z

To make it easier to track what is left to commit:

fcntl [does not apply cleanly
array
cmath [Martin says this no longer applies cleanly]

python-dev · 2014-10-10T20:26:41Z

New changeset f7ab0884467e by Brett Cannon in branch 'default':
Issue bpo-20152: Port the array module to Argument Clinic.
https://hg.python.org/cpython/rev/f7ab0884467e

brettcannon · 2014-10-10T20:27:11Z

Only ones left are:

fcntl
cmath

Both no longer apply cleanly.

python-dev · 2014-10-14T21:37:00Z

New changeset 5e8b94397f81 by Brett Cannon in branch 'default':
Issue bpo-20152: Convert the cmath module to Argument Clinic.
https://hg.python.org/cpython/rev/5e8b94397f81

brettcannon · 2014-11-07T17:44:18Z

Here is a new patch for fcntl. I would like a review since I had to get a bit tricky to handle the polymorphic arguments for fcntl and ioctl and I don't think the test suite is that thorough for this module.

serhiy-storchaka · 2014-11-07T19:32:51Z

I found bugs or doubtful code in the fcntl module. They should be fixed before converting to Argument Clinic.

brettcannon · 2014-11-08T14:28:38Z

So I disagree that the code needs to be tweaked before converting to Argument Clinic. If the Clinic conversion is not adding to the problem then the code churn is just going to make applying this patch that much harder.

Thanks for the code review regardless, though!

serhiy-storchaka · 2014-11-08T14:33:25Z

If first convert to Argument Clinic then fixing bugs will be much harder.

serhiy-storchaka · 2014-11-08T14:43:09Z

About argument names. You have changed argument names and docstrings in any case (e.g. was "op", now "code"). Why not conform with standard documentation? This wouldn't add additional code churn if change it now. But will add if change it later.

python-dev · 2014-11-10T01:23:02Z

New changeset 6e6532d313a1 by Brett Cannon in branch 'default':
bpo-20152, 22821: Port the fcntl module to Argument Clinic.
https://hg.python.org/cpython/rev/6e6532d313a1

brettcannon · 2014-11-10T01:26:34Z

Finally finished!

I am never trusting Larry to count anything ever again. ;)

brettcannon self-assigned this Jan 6, 2014

brettcannon added the extension-modules C modules in the Modules dir label Jan 6, 2014

larryhastings changed the title ~~Use Argument Clinic with _imp~~ Derby: Convert the _imp module to use Argument Clinic Jan 7, 2014

brettcannon assigned larryhastings and unassigned brettcannon Jan 7, 2014

larryhastings changed the title ~~Derby: Convert the _imp module to use Argument Clinic~~ Derby #15: Convert 50 sites to Argument Clinic across 9 files Jan 8, 2014

larryhastings added the type-feature A feature request or enhancement label Jan 8, 2014

brettcannon unassigned larryhastings Jan 10, 2014

brettcannon self-assigned this Jan 17, 2014

brettcannon assigned larryhastings and unassigned brettcannon Jan 17, 2014

brettcannon assigned brettcannon and unassigned larryhastings Jan 31, 2014

brettcannon assigned larryhastings and unassigned brettcannon Jan 31, 2014

brettcannon assigned brettcannon and unassigned larryhastings Jul 28, 2014

brettcannon closed this as completed Nov 10, 2014

ezio-melotti transferred this issue from another repository Apr 10, 2022

Derby #15: Convert 50 sites to Argument Clinic across 9 files #64351

Derby #15: Convert 50 sites to Argument Clinic across 9 files #64351

Comments

brettcannon commented Jan 6, 2014

brettcannon commented Jan 6, 2014

larryhastings commented Jan 7, 2014

brettcannon commented Jan 7, 2014

larryhastings commented Jan 8, 2014

python-dev mannequin commented Jan 10, 2014

brettcannon commented Jan 10, 2014

brettcannon commented Jan 17, 2014

brettcannon commented Jan 17, 2014

brettcannon commented Jan 17, 2014

brettcannon commented Jan 17, 2014

brettcannon commented Jan 21, 2014

brettcannon commented Jan 21, 2014

brettcannon commented Jan 31, 2014

brettcannon commented Jan 31, 2014

brettcannon commented Jan 31, 2014

loewis mannequin commented Jul 27, 2014

loewis mannequin commented Jul 27, 2014

python-dev mannequin commented Aug 22, 2014

brettcannon commented Aug 22, 2014

python-dev mannequin commented Aug 22, 2014

brettcannon commented Aug 22, 2014

python-dev mannequin commented Aug 22, 2014

python-dev mannequin commented Aug 22, 2014

python-dev mannequin commented Aug 22, 2014

brettcannon commented Aug 22, 2014

python-dev mannequin commented Oct 10, 2014

brettcannon commented Oct 10, 2014

python-dev mannequin commented Oct 14, 2014

brettcannon commented Nov 7, 2014

serhiy-storchaka commented Nov 7, 2014

brettcannon commented Nov 8, 2014

serhiy-storchaka commented Nov 8, 2014

serhiy-storchaka commented Nov 8, 2014

python-dev mannequin commented Nov 10, 2014

brettcannon commented Nov 10, 2014