Repository navigation
Integrate "Argument Clinic" into CPython trunk #60816
Copy link
Copy link
Closed
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancementA feature request or enhancement
Description
| BPO | 16612 |
|---|---|
| Nosy | @gvanrossum, @warsaw, @gpshead, @jcea, @mdickinson, @ncoghlan, @pitrou, @larryhastings, @ericvsmith, @jkloth, @ezio-melotti, @alex, @Trundle, @asvetlov, @skrah, @davidmalcolm, @meadori, @durban, @cjerdonek, @JimJJewett, @serhiy-storchaka, @koobs |
| 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/larryhastings'
closed_at = <Date 2013-10-19.07:10:52.670>
created_at = <Date 2012-12-04.22:18:57.517>
labels = ['interpreter-core', 'type-feature']
title = 'Integrate "Argument Clinic" into CPython trunk'
updated_at = <Date 2014-01-14.22:56:39.406>
user = 'https://github.com/larryhastings'bugs.python.org fields:
activity = <Date 2014-01-14.22:56:39.406>
actor = 'larry'
assignee = 'larry'
closed = True
closed_date = <Date 2013-10-19.07:10:52.670>
closer = 'larry'
components = ['Interpreter Core']
creation = <Date 2012-12-04.22:18:57.517>
creator = 'larry'
dependencies = []
files = ['28248', '29059', '29214', '29395', '29396', '29397', '32161', '32162', '32205', '32212', '32217', '32226']
hgrepos = []
issue_num = 16612
keywords = ['patch']
message_count = 73.0
messages = ['176965', '177127', '177148', '177156', '177162', '177165', '177167', '177168', '177169', '177233', '177234', '177235', '177263', '177264', '177265', '177295', '177298', '177336', '177357', '177359', '177376', '177446', '177495', '177497', '177499', '177500', '177501', '177502', '177505', '178479', '182000', '182030', '182046', '182053', '182054', '182060', '182067', '182084', '182220', '182221', '182234', '182238', '182268', '182842', '182978', '183002', '183023', '183039', '183040', '183045', '183047', '183847', '183849', '184075', '184134', '200181', '200182', '200258', '200293', '200295', '200297', '200306', '200310', '200342', '200380', '200381', '200382', '200384', '200448', '208119', '208120', '208121', '208122']
nosy_count = 27.0
nosy_names = ['gvanrossum', 'barry', 'gregory.p.smith', 'jcea', 'mark.dickinson', 'ncoghlan', 'pitrou', 'larry', 'eric.smith', 'jkloth', 'ezio.melotti', 'Arfrever', 'v+python', 'alex', 'Trundle', 'asvetlov', 'skrah', 'dmalcolm', 'meador.inge', 'daniel.urban', 'chris.jerdonek', 'python-dev', 'Jim.Jewett', 'serhiy.storchaka', 'bfroehle', 'koobs', 'david.villa']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue16612'
versions = ['Python 3.4']Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancementA feature request or enhancement
Activity
larryhastings commentedon Dec 4, 2012
This bug tracker entry is to track adding "Argument Clinic" to CPython trunk.
Please see http://mail.python.org/pipermail/python-dev/2012-December/122920.html for more information.
larryhastings commentedon Dec 7, 2012
Sorry for the delay; this head cold is slowing me down. Here's the current state of Argument Clinic as a patch for review. I look forward to your comments!
pitrou commentedon Dec 8, 2012
Can't you propose a PEP for this? I'd like to see the DSL officially discussed (or at least validated).
cjerdonek commentedon Dec 8, 2012
It seems the clinic.txt DSL document should be proofread for proper/consistent use of argument/parameter (e.g. as described in the recently added http://docs.python.org/dev/glossary.html#term-parameter ). To choose a couple random examples--
+Argument declaration | type name = default
+Argument flags | flag flag2 flag3=value
Shouldn't this be "Type declaration", etc?
+Functions With Positional-Only Arguments
+========================================
Positional-Only Parameters
skrah commentedon Dec 8, 2012
I second Antoine's request for a PEP. I'm not only concerned about the DSL:
This approach may work for modules like posixmodule.c, where each function
is largely a self-contained unit wrapping some API function.
On the other hand, I don't want to imagine how _decimal.c will look like:
There are > 100 functions that are closely related. Many of those functions
are *already* generated by macros. I think that vertical space is a precious
resource, so all docstrings (cruft as far as understanding the program is
concerned) are in a header file.
The clinic.py preprocessor would add at least 3000 lines to an already
large module, tearing apart groups of functions that form logical units.
Apologies for sounding negative, but IIRC this hasn't been brought up in
the python-dev discussion at all.
larryhastings commentedon Dec 8, 2012
Antoine, Stefan: There doesn't appear to be a bright line separating "this should get a PEP" from "this doesn't need a PEP". That said, changes to the C API for CPython don't seem to merit PEPs very often, the recent "Stable ABI" being the lone exception to the rule I can recall. Also, Guido already said (in python-dev) he didn't think it merited a PEP, and I tend to agree.
What I don't understand is what advantage writing a PEP would bring. If you're worried that it needs discussion--okay, let's discuss it. Surely we can do that here? Or in python-dev? What's on your mind?
Stefan: It sounds like Argument Clinic won't work well for _cdecimal, nor would any alternate DSL grammar work any better, really. But have no fear--I doubt anyone is going to force you to accept Argument-Clinic-ization of _cdecimal against your will. If you're comfortable maintaining the _cdecimal code without it, you certainly have my blessing. Though I do hope you'll add the data needed to generate Signature objects once we figure out what that looks like.
Chris: clinic.txt is available for review as part of the patch I posted, please feel free to make your comments there. I could file a new patch with the two changes you already suggest if you prefer. (Though I'm not changing the name of the project to "Parameter Clinic"--the Monty Python reference is simply irresistible.)
Finally, an admission. I wrote "Argument Clinic" thinking that the inspect.Signature Parameter objects had a per-Parameter docstring. Well, they don't. (I feel like an idiot.) So we could theoretically remove the per-parameter docstrings from the DSL. However, I'm pretty happy with the current structure of the DSL. My inclination therefore is to leave them in. (And who knows, maybe the Parameter object could grow a docstring.) Still, if you're going to propose changes to the DSL, I submit that removing the per-parameter docstring is wholly viable.
pitrou commentedon Dec 8, 2012
My argument for requiring a PEP is that this change will not only affect
the people maintaining the clinic code. It will affect everyone
contributing and maintaining code in the CPython codebase. It's much
more than "just an addition to the C API", it's a change in how CPython
is developed.
Writing and proposing a PEP brings public scrutiny in a much more
visible, and also better-recorded, way than a bug entry on a tracker.
Note that writing a PEP doesn't mean that there'll be a huge discussion
about it. And you needn't post it on python-ideas, you can post it on
python-dev instead.
alex commentedon Dec 8, 2012
For what it's worth, I'm not as concerned with the process of the PEP, as having a single document we can review and discuss.
pitrou commentedon Dec 8, 2012
(and, oh, I agree there isn't a bright line separating "this should get
a PEP" from "this doesn't need a PEP")
skrah commentedon Dec 9, 2012
Here are some things that I'm interested in right now:
From what kind of data structure are signature objects generated?
Is there an easy way to do this manually?
Can at least part of the verboseness go into header files (virtually
all compilers support "static inline" in header files)?
Why don't approaches work that look simpler on the surface, like
http://mail.python.org/pipermail/python-dev/2012-December/122934.html ?
Are C contributors in general happy (or at least tolerate) the new
verboseness?
Sure, we can discuss 1) to 3) here. For recording objections or approaches
that don't work (think FAQ "Why don't you just do X?") a PEP would be nice
to have.
If I'm the only one who is slightly bothered by the aesthetic and readability
aspects, then 5) obviously need not be discussed. Most people, however, will
probably find out about the change only once Python 3.4 is released.
larryhastings commentedon Dec 9, 2012
I disagree that the Clinic DSL is "verbose". Certainly I find it
more succinct than what we do now.
On the other hand, the syntax you proposed in the python-dev message
you cite is insufficient to the task. Consider a function that
takes a "char *". How might you want Python to convert the Python
value into that char *? Should it only accept bytes? Should it
accept bytes and Unicode? Should it accept objects exposing the
buffer protocol? Do you want the length too? If you want the
length, does that mean that you accept nulls inside the string?
If it's Unicode, do you want to use a default encoding, or do you
want to specify an encoding? Or do you want to specify your own
conversion function?
My solution for the above is to add "flags". Which is,
semantically, the only important distinction between what you
propose and Clinic's DSL. (That and the per-argument docstrings,
which I already suggest are negotiable.) The remaining differences
are that you want to cram everything onto one line, wheras Clinic's
DSL spreads things over multiple lines. I suggest the latter is
more readable, particularly when the number of arguments grows
large.
To answer 5), you're the only C contributor who comes to mind who
seems unhappy. Do I understand you correctly that your main pain
point is that you generate scores of identical functions with the
C preprocessor, and you see no way to do that with Clinic? If we
figured out a way to make that work with Clinic, would that reduce
your concerns?
pitrou commentedon Dec 9, 2012
Le dimanche 09 décembre 2012 à 19:20 +0000, Stefan Krah a écrit :
You're not the only one. The vertical space argument also resonates with
me, although I'm not sure how easy it would be to put the generated
boilerplate into separate include files.
The DSL is a bit weird, it doesn't seem to have consistent typography
rules, some lines end with a colon, some lines don't, some assignment
signs ("=") have spaces around them, some don't.
54 remaining items
serhiy-storchaka commentedon Oct 18, 2013
Clinic can define a macro with a random value in a main file before including generated file and check this value by preprocessor in generated file. It will guard us from unintentional desynchronization.
serhiy-storchaka commentedon Oct 18, 2013
I have added a handful of nitpicks on Rietveld.
pitrou commentedon Oct 18, 2013
That's only good if editing the file needs 3GB RAM.
larryhastings commentedon Oct 19, 2013
Incorporated a small change from Antoine and a ton of small changes
from Serhiy. Thanks guys!
python-dev commentedon Oct 19, 2013
New changeset 8fde1a2c94dc by Larry Hastings in branch 'default':
Issue bpo-16612: Add "Argument Clinic", a compile-time preprocessor
http://hg.python.org/cpython/rev/8fde1a2c94dc
larryhastings commentedon Oct 19, 2013
Checked in. Thanks everybody!
koobs commentedon Oct 19, 2013
_curses and _curses_panel fail to build after this commit on koobs-freebsd10 buildslave, with some lovely clang warnings too.
Attaching full log here.
python-dev commentedon Oct 19, 2013
New changeset 47618b00405b by Serhiy Storchaka in branch 'default':
Fix compilation of the curses module (broken by issue bpo-16612).
http://hg.python.org/cpython/rev/47618b00405b
serhiy-storchaka commentedon Oct 19, 2013
Here is a patch with yet some tiny changes (but they are too large for just comment). It corrects misleading docstrings of permute_*_option_groups() and simplifies rstrip_lines() and permute_optional_groups(). Fill free to apply those of them which look good to you.
python-dev commentedon Jan 14, 2014
New changeset f35b3a86ade3 by Meador Inge in branch 'default':
Fix minor bug in dict.__contains__ docstring.
http://hg.python.org/cpython/rev/f35b3a86ade3
larryhastings commentedon Jan 14, 2014
Meador Inge: In the future, please create new issues for these sorts of fixes.
meadori commentedon Jan 14, 2014
Will do, but in this case I didn't think a one character diff was worth
it.
larryhastings commentedon Jan 14, 2014
If it's not worth a new issue, then you can check it in without citing an issue at all.