| msg208395 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2014-01-18 11:43 |
| Currently Argument Clinic doesn't support the __init__ methods, mainly because the C implementation of this method should return int, not PyObject *. In some cases it is possible to wrap a function generated by Argument Clinic (as in Modules/_pickle.c). But not always, because the __init__ method is called with the self, args, and kwargs arguments, while Argument Clinic can generate function without kwargs. |
|
|
| msg208406 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-18 17:41 |
| Oh! I thought it worked fine, because I've seen people convert both an __init__ and a __new__. But I guess they wrapped them. It's an easy change, I can try to do that today. |
|
|
| msg208412 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-18 19:36 |
| > * As is probably expected, __init__ and __call__ procs > can't be converted. I should have a fix in for __init__ and __call__ later today. (Issue #20294.) For the record, you could convert them, you just had to wrap the parsing function to deal with the return type difference, and *args / **kwargs might be tricky too. > * sqlite3.Connection.{execute, executemany, executescript} don't use > PyArg_Parse*. In the next few days I'm going to add support for "*args" and "**kwargs", and then you'll be able to convert these functions. (Issue #20291.) Again, even if Argument Clinic doesn't do any parsing, it's still helpful to have it generate signatures. > * The clinic version of 'sqlite3.adapt' has an argument string of "O|OO". > The first optional parameter defaults to 'pysqlite_PrepareProtocolType' > in C and 'sqlite3.ProtocolType' in Python. However, I don't know how to > represent the Python value because Python default values are 'eval'd by > clinic *and* the 'sqlite3' module is not imported for the eval. You'll be able to do this soon: parameter: object(c_default='pysqlite_PrepareProtocolType') = ProtocolType This feature isn't checked in yet, I'm waiting for a review. It's part of #20226, which I guess you already noticed. > * The clinic version of 'sqlite3.Cursor.fetchmany' has an arguments string > of " |
i". The first parameter defaults to > '((pysqlite_Cursor *)self)->arraysize' in C and 'self.arraysize' in > Python. I don't know how to express the Python initialization. You can't. How about you use a default of -1 and then: if (maxrows == -1) maxrows = self->arraysize > * I didn't convert 'sqlite3.Cursor.setinputsizes' and > 'sqlite3.Cursor.setoutputsize' because they share a docstring and > simple no-op method def. I'd prefer it if you converted them anyway. Converting them means they'll have signature information which is an unconditional good. > * I didn't convert 'sqlite.connect' because it would have required > packaing the arguments back up to pass to 'PyObject_Call'. Once I add the ability to pass in *args and **kwargs, you'll be able to convert sqlite.connect. I think I responded to all your other comments when I reviewed the patch. Thanks! |
|
| msg208413 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-18 19:37 |
| Oops, sorry, that last comment was intended for a different issue. Please ignore, and sorry for the spam! |
|
|
| msg208444 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-19 04:39 |
| Here's a patch. Wasn't as easy as I thought, it kind of took all day. Changes include: * __init__ and __new__ always take kwargs. * if the function signature doesn't accept keyword arguments, it calls _PyArg_NoKeywords(). * __init__ returns int, not PyObject *. * __init__ and __new__ should support all argument parsing scenarios (unpack tuple, positional only, etc). Pre-exiting behavior: * The C function basename chosen for __new__ is just the name of the class, it doesn't end in __new__. * The methoddef #define is suppressed. |
|
|
| msg208451 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2014-01-19 05:59 |
| Rietveld doesn't like the patch and I'm having a hard time finding a changeset on which I can get it to apply cleanly. May I suggest avoiding `--git` on `hg diff` unless copying/moving/renaming a file? |
|
|
| msg208453 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-19 06:10 |
| Whoops, that doesn't apply cleanly. Here's an updated patch. |
|
|
| msg208458 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2014-01-19 07:20 |
| Output looks good and I didn't see anything that scared me in the patch, though I can't pretend to fully understand 100% of it. Builds on Windows, pickle tests pass, and nothing else is affected (that's checked in so far). Looks good to me, as far as I can see. |
|
|
| msg208459 - (view) |
Author: Ryan Smith-Roberts (rmsr) * |
Date: 2014-01-19 07:23 |
| I have reviewed this as best I am able. I'll be honest that a lot of clinic.py makes my eyes cross; I'm used to webdev templating, which is inverted from AC (flow control inside the template). Not complaining, it's a complicated subject. I do like the centralization of the templates. I didn't see anything that jumped out at me. I tried out the new conversions on socketmodule and like the output, and none of the existing conversions mutated. So LGTM! It took me a while to figure out how to make __new__ a class method though, decorator support should probably be documented (or else a hint like '__new__ must be a @classmethod!') |
|
|
| msg208460 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2014-01-19 07:50 |
| New changeset 2e32462e4832 by Larry Hastings in branch 'default': Issue #20294: Argument Clinic now supports argument parsing for __new__ and http://hg.python.org/cpython/rev/2e32462e4832 |
|
|
| msg208461 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-19 07:51 |
| Checked in. Thanks for bringing the problem to my attention, Serhiy, and thanks for the reviews you two! rmsr: If I had a stronger background in templating, maybe Argument Clinic would be cleaner inside. As it is I'm beating the problem to death with sheer force of will. |
|
|
| msg208497 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-01-19 20:13 |
| Thank you Larry, now I can use Argument Clinic in the __init__ methods. But there is one problem. Docstring generated for the __init__ method contains the "__init__" name in the signature. Therefore it can't be used as class docstring. On other hand, the compiler complains of not used __init__ docstring. |
|
|
| msg208739 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-22 03:13 |
| That problem will be irrelevant, once builtin classes support signatures (#20189). |
|
|