Title: Argument Clinic rollup patch, 2014/01/25
Type: enhancement Stage: resolved
Components: Demos and Tools Versions: Python 3.4
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: larry, ncoghlan, python-dev, serhiy.storchaka, zach.ware
Priority: normal Keywords:

Created on 2014-01-25 15:28 by larry, last changed 2014-01-31 13:59 by python-dev. This issue is now closed.

File name Uploaded Description Edit larry, 2014-01-25 15:28 larry, 2014-01-26 00:11
Messages (15)
msg209205 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-25 15:28
Rollup patch with a bunch of small fixes in it.  Can I get a quick turnaround review on this?  I'd like it to go in before today's beta is cut.  Definitely the core change has to go in, but that's uncontroversial.

  * _PyType_GetDocFromInternalDoc() and
    _PyType_GetTextSignatureFromInternalDoc() are now marked

  * The default filename for the "file" preset is now:

  * The "class" directive syntax has been extended with two new required
    arguments: "typedef" and "type_object".  "typedef" should be the
    equivalent C type for this Python class, presented as a
    pointer-to-type.  "type_object" should be an expression that
    evaluates to a PyTypeObject * for this Python type.  Example:
        class dict "PyDictObject *" "&PyDict_Type"
    This was necessary for the next item.

  * If __new__ or __init__ functions did not use kwargs (or args), the
    PyArg_NoKeywords (or PyArg_NoPositional) calls generated are only
    run if the type object is an exact match.  (Otherwise subclasses
    will not be allowed to start accepting those kinds of arguments.)

  * Argument Clinic now fails if you have required parameters after
    optional parameters.

  * Converters now have a new template they can inject code into:
    it's called "modifiers", and code put there is run in the parsing
    function after argument parsing but before the call to the impl.
    "modifiers" is intended for converters that may want dynamic
    default values.  An example string you might add:
        "if ({name} == NULL)\n    name = time(NULL);"
msg209210 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-01-25 15:47
LGTM, but I'm not familiar with Clinic's own code...
msg209216 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-25 16:29
Don't forget to use "hg mv" to rename existing side files.
msg209230 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-25 19:50
_PyType_GetDocFromInternalDoc and _PyType_GetTextSignatureFromInternalDoc declarations are too long. could you please wrap them around?

First parameter of class methods (e.g. dict.fromkeys) now has a type of object (PyDictObject *) instead of PyTypeObject * (or PyDict_Type *).

There is redundant cast in dict___contains__:

    register PyDictObject *mp = (PyDictObject *)self;

First parameter of static methods (e.g. str.maketrans) now has a type of object (PyUnicodeObject *) instead of void *.

There is redundant cast in unicodedata_UCD_decimal_impl:

    PyUnicodeObject *v = (PyUnicodeObject *)unichr;

Perhaps here should be os.path.dirname() instead of os.path.basename():

    dirname = os.path.basename(destination.filename)

The 'fail("Can't write to destination "' line is too long. And I think it will look better if use string formatting.
msg209254 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-25 23:49
Thanks for the reviews, Nick and Serhiy, especially considering that the Rietveld link didn't work!

Serhiy: I believe I fixed everything.  There are no side files checked in yet.  Can you give me another review?
msg209255 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-25 23:51
Hang on, my tree was out of date.  Which is why I didn't get a review link, duh.  And if side files have appeared I'll fix 'em.
msg209257 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-26 00:11
Okay, here's my revised patch, for real this time, with "hg mv" for the side files and everything.
msg209259 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-26 00:21
Oops, I goofed up moving  I'll fix that locally, won't regenerate the patch just for that (unless you ask me to).
msg209260 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-26 00:22
No, wait, I think it's fine, it was just some detritus in my directory throwing me off.  But "hg stat" looks correct.
msg209275 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-26 04:22
I don't know why it's refusing to generate the review link.  The patch applies cleanly to trunk, even now, hours later.  Maybe it doesn't like file moves?
msg209276 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-26 04:44
New changeset 9c5b421cc7de by Larry Hastings in branch 'default':
Issue #20390: Small fixes and improvements for Argument Clinic.
msg209279 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-26 06:01
New changeset 1435d2fe8e34 by Larry Hastings in branch 'default':
Issue #20390: Final fix, for generating NoPositional/NoKeyword for __init__ calls.
msg209282 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-26 07:13
Rietveld doesn't like patches against out of date tree. And unfortunately it doesn't like patches in git format.
msg209283 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-26 07:15
I don't know about diff #1, but diff #2 definitely applied cleanly against a fresh trunk at 8:22pm local time.  I've turned off git format.
msg209790 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-31 13:59
New changeset 06b4fbb09c93 by Larry Hastings in branch 'default':
Issue #20390: Removing detritus from Argument Clinic "file preset" name change.
Date User Action Args
2014-01-31 13:59:58python-devsetmessages: + msg209790
2014-01-29 20:48:17yselivanovlinkissue20231 dependencies
2014-01-26 07:15:28larrysetmessages: + msg209283
2014-01-26 07:13:35serhiy.storchakasetmessages: + msg209282
2014-01-26 06:01:38python-devsetmessages: + msg209279
2014-01-26 04:45:25larrysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2014-01-26 04:44:02python-devsetnosy: + python-dev
messages: + msg209276
2014-01-26 04:22:46larrysetmessages: + msg209275
2014-01-26 00:22:25larrysetmessages: + msg209260
2014-01-26 00:21:31larrysetmessages: + msg209259
2014-01-26 00:11:18larrysetfiles: +

messages: + msg209257
2014-01-25 23:51:21larrysetmessages: + msg209255
2014-01-25 23:49:22larrysetfiles: -
2014-01-25 23:49:06larrysetfiles: +

messages: + msg209254
2014-01-25 19:50:03serhiy.storchakasetmessages: + msg209230
2014-01-25 16:29:00serhiy.storchakasetmessages: + msg209216
2014-01-25 15:47:33ncoghlansetmessages: + msg209210
2014-01-25 15:28:46larrycreate