[issue15831] comma after leading optional argument is after bracket in docs
New submission from Chris Jerdonek:
From docs@python.org:
""" In a number of places we find documentation with optional leading arguments where the meta-notation (square brackets) are in the wrong place. For example, for range, the standard doc heading says range([start], stop [, step]) # The comma after start should be inside the square bracket, not outside. The same error occurs with docs for print, and for slice, for random.randrange and for random.seed, etc, etc. This seems a consistent bug whenever a function or method takes an optional first argument, and it looks like it might have been put there by some tool that generates the docs. I use Python 3 - perhaps there was once a time when one had to supply the comma if one omitted the first argument? In Python 3 it gives errors if one follows the documentation carefully and uses the comma. """" (or see http://mail.python.org/pipermail/docs/2012-August/010051.html ) ---------- assignee: docs@python components: Documentation messages: 169547 nosy: cjerdonek, docs@python priority: normal severity: normal status: open title: comma after leading optional argument is after bracket in docs type: behavior versions: Python 2.7, Python 3.2, Python 3.3 _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Changes by Ezio Melotti <ezio.melotti@gmail.com>: ---------- keywords: +easy nosy: +ezio.melotti stage: -> needs patch versions: +Python 3.4 _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Changes by Andrew Svetlov <andrew.svetlov@gmail.com>: ---------- nosy: +asvetlov _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Do we know that this is easy? It might be a Sphinx issue, in which case it might not be as trivial as fixing typos. I ask because the reST file looks correct (for range() for example): .. function:: range([start,] stop[, step]) ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment: I thought that was just a matter of finding the wrong commas and fixing them, but if they are correct in the source, then the situation might be a bit more complicated. Does this happen only with "unusual" signatures like range([start], stop [, step])? FWIW this could be replaced with: range(stop) range(start, stop [, step]) ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: This is what my search for ",] " gave me (though the OP's print and random.seed do not show up in this list): library/curses.rst 380:.. function:: newwin([nlines, ncols,] begin_y, begin_x) 659:.. method:: window.addch([y, x,] ch[, attr]) 673:.. method:: window.addnstr([y, x,] str, n[, attr]) 679:.. method:: window.addstr([y, x,] str[, attr]) 766:.. method:: window.chgat([y, x, ] [num,] attr) 815:.. method:: window.derwin([nlines, ncols,] begin_y, begin_x) 909:.. method:: window.hline([y, x,] ch, n) 943:.. method:: window.insch([y, x,] ch[, attr]) 964:.. method:: window.insnstr([y, x,] str, n [, attr]) 1156:.. method:: window.subpad([nlines, ncols,] begin_y, begin_x) 1162:.. method:: window.subwin([nlines, ncols,] begin_y, begin_x) 1219:.. method:: window.vline([y, x,] ch, n) library/functions.rst 1064:.. function:: range([start,] stop[, step]) 1129:.. function:: slice([start,] stop[, step]) library/itertools.rst 55::func:`islice` seq, [start,] stop [, step] elements from seq[start:stop:step] ``islice('ABCDEFG', 2, None) --> C D E F G`` 404:.. function:: islice(iterable, [start,] stop [, step]) library/random.rst 96:.. function:: randrange([start,] stop[, step]) library/syslog.rst 20:.. function:: syslog([priority,] message) library/tkinter.tix.rst 507:.. method:: tixCommand.tix_configure([cnf,] **kw) ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Andrew Svetlov added the comment: Yes, it looks like Sphinx problem. About having tho signatures for single function/method. Does Sphinx support it for *.. function::*? Pointing to anchor for first signature with adding second one somewhere in paragraph text doesn't look good. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment:
Does Sphinx support it for *.. function::*?
I'm pretty sure it does for methods, so I don't see why it shouldn't work for functions. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Andrew Svetlov added the comment: No, methods for *curses* are also corrupted. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment:
This is what my search for ",] " gave me
It would be fine with me to use a double signature for these. Georg, do you have any opinion? (The double signature might be easier to understand, but the original issue should probably be fixed in Sphinx, even if we decide to stop using this kind of signature.) ---------- nosy: +eric.araujo, georg.brandl _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment:
No, methods for *curses* are also corrupted.
I meant the double signature support. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment:
It would be fine with me to use a double signature for these.
Just an FYI that more than two signatures would be needed for cases like this: 766:.. method:: window.chgat([y, x, ] [num,] attr) ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment:
766:.. method:: window.chgat([y, x, ] [num,] attr)
What's this even supposed to mean? See also #14783. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment:
What's this even supposed to mean?
I started wondering the same thing after I posted. :) I guess my point/question is: are there any cases where more than two signatures would be needed to account for all of the possibilities? ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment: I think/hope that all the APIs we have in the stdlib are sane enough to have no more than 2-3 signatures (I'm not counting optional args (at the end) here). If that's not the case we should still be able to add as many signature as we need (I don't know if Sphinx has some limit about it though). ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Georg Brandl added the comment: +1 for multiple signatures. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment:
(The double signature might be easier to understand, but the original issue should probably be fixed in Sphinx, even if we decide to stop using this kind of signature.)
I filed an issue for this in the Sphinx tracker here: https://bitbucket.org/birkenfeld/sphinx/issue/1001/comma-after-leading-optio... ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: I will prepare a patch (multiple signatures, for the Python fix). ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Something to be aware of that may or may not affect the patch I'm preparing: One reason that Sphinx seems able to render some of the more complicated function signatures is that it has logic to bail and print the parameter list verbatim from the reST file whenever its record-keeping logic becomes internally inconsistent: except IndexError: # if there are too few or too many elements on the stack, just give up # and treat the whole argument list as one argument, discarding the # already partially populated paramlist node https://bitbucket.org/birkenfeld/sphinx/src/1f3a2749df39/sphinx/domains/pyth... This seems to come into play, for example, when rendering-- sorted(iterable[, key][, reverse]) http://docs.python.org/dev/library/functions.html#sorted ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Here are a couple functions that may need a fix different from multiple signatures: print([object, ...], *, sep=' ', end='\n', file=sys.stdout, flush=False) http://docs.python.org/dev/library/functions.html#print class argparse.ArgumentParser([description][, epilog][, prog][, usage][, add_help][, argument_default][, parents][, prefix_chars][, conflict_handler][, formatter_class]) http://docs.python.org/dev/library/argparse.html#argparse.ArgumentParser ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: FWIW, I submitted a patch for the Sphinx issue I created: https://bitbucket.org/birkenfeld/sphinx/issue/1001 Ironically, that patch was probably easier than this patch will be. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Attaching a proposed patch for the default branch. Also, here are several comments and questions.
I think/hope that all the APIs we have in the stdlib are sane enough to have no more than 2-3 signatures
I found this one in the curses module with four: window.chgat(attr) window.chgat(num, attr) window.chgat(y, x, attr) window.chgat(y, x, num, attr) Do we like how these look? Is the bare star notation too obscure? inspect.Signature.replace(*[, parameters][, return_annotation]) inspect.Parameter.replace(*[, name][, kind][, default][, annotation]) I was curious what the preferred way to display the following is, since I don't think any comma/bracket placement will work: ArgumentParser([description][, epilog][, prog][, usage][, add_help][, argument_default][, parents][, prefix_chars][, conflict_handler][, formatter_class]) (unless perhaps we use the construction "ArgumentParser(*[, description][, epilog]....") I'm not sure how we want to handle this one using multiple signatures: multiprocessing.Process([group[, target[, name[, args[, kwargs]]]]], *, daemon=None) I put my preferred rendering in the patch, but Sphinx re-renders it in its own way. I also noticed these more unusual signatures: urllib.request.urlopen(url, data=None[, timeout], *, cafile=None, capath=None, cadefault=True) http.client.HTTPSConnection(host, port=None, key_file=None, cert_file=None[, strict[, timeout[, source_address]]], *, context=None, check_hostname=None) By the way, is the * really necessary in these examples? ---------- keywords: +needs review, patch stage: needs patch -> patch review Added file: http://bugs.python.org/file27104/issue-15831-1.patch _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment:
Do we like how these look? Is the bare star notation too obscure?
inspect.Signature.replace(*[, parameters][, return_annotation]) inspect.Parameter.replace(*[, name][, kind][, default][, annotation])
Note that if possible, it's better to avoid using the [] and put the default values. However, in these cases the default value seems to be a _void placeholder. The star notation is OK, since it's valid Python.
I was curious what the preferred way to display the following is, since I don't think any comma/bracket placement will work:
ArgumentParser([description][, epilog][, prog][, usage][, add_help][, argument_default][, parents][, prefix_chars][, conflict_handler][, formatter_class])
If the default values are known you could use them, otherwise this signature looks OK to me.
(unless perhaps we use the construction "ArgumentParser(*[, description][, epilog]....")
This is OK too, as long as all the arguments (including 'description') are keyword-only.
I'm not sure how we want to handle this one using multiple signatures:
multiprocessing.Process([group[, target[, name[, args[, kwargs]]]]], *, daemon=None)
This is OK too.
I also noticed these more unusual signatures:
urllib.request.urlopen(url, data=None[, timeout], *, cafile=None, capath=None, cadefault=True) http.client.HTTPSConnection(host, port=None, key_file=None, cert_file=None[, strict[, timeout[, source_address]]], *, context=None, check_hostname=None)
Here the args between [] probably have a placeholder default, or are obtained from somewhere else (e.g. from socket.getdefaulttimeout()). These signature, albeit a bit weird, are understandable IMHO, so, unless you find a better way to write them, I would leave them as they are.
By the way, is the * really necessary in these examples?
If the args on the right of the * are keyword-only and the one on the left aren't, then yes. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Thanks for responding to all of those questions, Ezio. I will update the patch based on your responses. (Likely most of it will remain the same.)
Note that if possible, it's better to avoid using the [] and put the default values. However, in these cases the default value seems to be a _void placeholder.
Yes, the defaults are "private" in those cases (and in I think pretty much all cases in the patch in which []'s rather than a default value are used to display keyword arguments).
I was curious what the preferred way to display the following is, since I don't think any comma/bracket placement will work:
ArgumentParser([description][, epilog][, prog][, usage][, add_help][, argument_default][, parents][, prefix_chars][, conflict_handler][, formatter_class])
If the default values are known you could use them, otherwise this signature looks OK to me.
The comma not being inside the first bracket is what I was concerned about here (which was the issue raised in the original comment). Using an initial * which you are okay with will address this. (And yes, they are all keyword arguments. In fact, the order of the arguments in the docs does not even match the order in the code.)
By the way, is the * really necessary in these examples?
If the args on the right of the * are keyword-only and the one on the left aren't, then yes.
Okay. I think what threw me is that I don't think we're consistently using the * throughout our docs to denote the beginning of keyword-only arguments. The ArgumentParser constructor above is just one example. So I wasn't sure if this notation was preferred or discouraged. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment:
So I wasn't sure if this notation was preferred or discouraged.
We are moving toward using the same signatures that we have in the Python code, but that doesn't always work (e.g. when the meaning of values changes depending on the number of args, when there's some trick with *args/**kwargs involved, when there are default placeholders, etc.). The single * is not so common (yet), and it might be difficult to understand for new users, that's why I said it's OK to leave it out in cases where all the args are keyword args and the [] are used. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Attaching an updated patch after doing another pass at the code and in light of Ezio's comments. Let me know if and when you'd like me to prepare separate patches for 2.7 and 3.2. ---------- Added file: http://bugs.python.org/file27120/issue-15831-2.patch _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment:
We are moving toward using the same signatures that we have in the Python code.... The single * is not so common (yet),
I created issue 15865 to add the single * to the documentation where appropriate. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Roundup Robot added the comment: New changeset b01eb870f84f by Ezio Melotti in branch '3.2': #15831: document multiple signatures on different lines. Patch by Chris Jerdonek. http://hg.python.org/cpython/rev/b01eb870f84f New changeset 0ed61ee823d8 by Ezio Melotti in branch 'default': #15831: merge with 3.2 http://hg.python.org/cpython/rev/0ed61ee823d8 New changeset 2291aff20a05 by Ezio Melotti in branch '2.7': #15831: document multiple signatures on different lines. Patch by Chris Jerdonek. http://hg.python.org/cpython/rev/2291aff20a05 ---------- nosy: +python-dev _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment: Fixed, thanks for the patch! ---------- assignee: docs@python -> ezio.melotti resolution: -> fixed stage: patch review -> committed/rejected status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Thanks, Ezio! By the way, I didn't do a thorough check, but I noticed this difference in the 2.7 application of the patch. The *key* argument for max() needs to be marked keyword-only. This difference doesn't exist for the min() function. Or are we not using the "bare *" notation in the 2.7 docs? If not, the min() docs need to be changed instead (and possibly in several other places). Default branch: -.. function:: max(iterable[, args...], *[, key]) +.. function:: max(iterable, *[, key]) + max(arg1, arg2, *args[, key]) 2.7 branch: -.. function:: max(iterable[, args...][key]) +.. function:: max(iterable[, key]) + max(arg1, arg2, *args[, key]) ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Someone pointed out that keyword-only arguments were introduced only in 3.0, but I'm not sure whether that means we can't use them as a notational device in the 2.7 docs. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Chris Jerdonek added the comment: Since the bare * notation wasn't added until 3.0, my guess is that we want to remove the * below (from the 2.7 application of the patch) rather than adding it back in the max() function I pasted above: -.. function:: min(iterable[, args...][key]) +.. function:: min(iterable, *[, key]) + min(arg1, arg2, *args[, key]) ---------- resolution: fixed -> stage: committed/rejected -> needs patch status: closed -> open _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Roundup Robot added the comment: New changeset 881acdf9133f by Ezio Melotti in branch '2.7': #15831: avoid using 3.x syntax for keyword-only args. http://hg.python.org/cpython/rev/881acdf9133f ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
Ezio Melotti added the comment: The * is not supposed to be used on 2.7, but I missed that. Thanks for noticing! ---------- resolution: -> fixed stage: needs patch -> committed/rejected status: open -> closed type: behavior -> enhancement _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue15831> _______________________________________
participants (5)
-
Andrew Svetlov -
Chris Jerdonek
-
Ezio Melotti
-
Georg Brandl
-
Roundup Robot