classification
Title: Add docstrings for ElementTree module
Type: enhancement Stage: patch review
Components: Documentation, XML Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: dlam, docs@python, eli.bendersky, ezio.melotti, python-dev, serhiy.storchaka, tshepang
Priority: normal Keywords: easy, patch

Created on 2013-01-13 15:26 by serhiy.storchaka, last changed 2013-04-14 21:47 by eli.bendersky.

Files
File name Uploaded Description Edit
issue16954.patch dlam, 2013-02-17 05:22 review
issue16954-v2.patch dlam, 2013-02-28 10:19
Messages (15)
msg179881 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-13 15:26
Perhaps almost all Doxygen comments in ElementTree module should be converted to docstrings.
msg179883 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-01-13 17:17
Definitely. And this is one of those issues where I can wholeheartedly say that patches are welcome :-)

Even incremental patching will be OK (i.e. patches documenting single methods or groups of methods).

Incidentally, while replacing the comment by docstring for issue #14377, I noticed an argument (default_namespace) that wasn't documented anywhere, including the ReST docs. So such a transition can have more benefits than seems on the surface.
msg180536 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-01-24 18:02
I would suggest to adapt the comments to follow PEP 257, and in particular:
"""
The docstring is a phrase ending in a period. It prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ...".
"""

Currently there are two docstrings, one for write() and one for iterparse() (recently added in #9708).  Only the former uses the correct form ("Return" instead of "Returns").
msg180538 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-01-24 18:15
On Thu, Jan 24, 2013 at 10:02 AM, Ezio Melotti <report@bugs.python.org>wrote:

>
> Ezio Melotti added the comment:
>
> I would suggest to adapt the comments to follow PEP 257, and in particular:
> """
> The docstring is a phrase ending in a period. It prescribes the function
> or method's effect as a command ("Do this", "Return that"), not as a
> description; e.g. don't write "Returns the pathname ...".
> """
>
> Currently there are two docstrings, one for write() and one for
> iterparse() (recently added in #9708).  Only the former uses the correct
> form ("Return" instead of "Returns").
>

Actually, the latter was copy-pasted from the ReST docs of the method. Does
that PEP 257 suggestion apply to ReST docs as well, or do we have a
discrepancy?
msg180540 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-01-24 18:19
The docs should use "return" too, even though I'm not sure this is enforced.  Consistency within the doc page is more important, but I don't think that consistency between comments and docstrings in the code or between docstrings and documentation is so important.
msg180544 - (view) Author: David Lam (dlam) * Date: 2013-01-24 19:39
I had an innocent question about the format to use when listing function arguments in docstrings.  In the PEP 257 doc, there's a single example:

    def complex(real=0.0, imag=0.0):
        """Form a complex number.

        Keyword arguments:
        real -- the real part (default 0.0)
        imag -- the imaginary part (default 0.0)

I went digging through Lib/ for a good example to follow, but felt a little unsure because the exact format seemed to differ ever so slightly sometimes.

Like in ipaddress.py, a colon is used instead of two hyphens, and it's indented:

    def ip_address(address):
        """Take an IP string/int and return an object of the correct type.

        Args:
            address: A string or integer, the IP address.  Either IPv4 or
              IPv6 addresses may be supplied; integers less than 2**32 will
              be considered to be IPv4 by default.

Is there an "ideal" example in the source to try to copy?  
(or maybe this is just a use-your-common-sense thing)
msg180545 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-01-24 19:43
> (or maybe this is just a use-your-common-sense thing)

That's probably the best thing.  I don't think we follow any specific convention for args in the docstring.  Mostly they are just described in the text, without having lists of args.
msg182261 - (view) Author: David Lam (dlam) * Date: 2013-02-17 05:22
Here's a patch which converts all the Doxygen comments in ElementTree.py to docstrings!

Something I noticed was that the 

  from _elementtree import *

...at the bottom of ElementTree.py sort of overwrites the docstrings of the Python module.  So if you did...  `from xml.etree import ElementTree;  help(ElementTree)` from the interpreter, you'll see blank spots for a bunch of classes/methods like Element, ParseError, SubElement etc etc

maybe docstrings should be also added in Modules/_elementtree.c?  perhaps that would be too much copy and pastage, hmmm
msg182542 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-02-20 18:58
I left a few comments on rietveld.

In the rst docs the markup used for arguments is *arg*, and this is sometimes reflected in docstrings too.  We might want to do this here too, instead of using 'arg' (using 'attr' for attributes it's fine though).

> maybe docstrings should be also added in Modules/_elementtree.c?

Maybe we could have a loop that does something like cfunc.__doc__ = pyfunc.__doc__?
msg183210 - (view) Author: David Lam (dlam) * Date: 2013-02-28 10:19
here's an updated patch incorporating the feedback from Ezio and Eric:

   - moved docstrings put in some __special__ method names
   - made the description of 'tag' consistent:  'tag' means the elements name (as opposed to 'tag' being a synonym for "element"!)
   - docstring args now *stared* as opposed to 'quoted'


I also gave a shot at copying the existing docstrings into their respective C counterparts.  But it *seems* like you can't do so
that easily for a C extension. Maybe I missed something though: 

>>> from _elementtree import Element as cElement
>>> cElement.__doc__ = 'foobarbaz'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't set attributes of built-in/extension type 'xml.etree.ElementTree.Element'

I see one example in Modules/_json.c where the docstrings were sorta copied and pasted over, so perhaps it's an ok thing to do.
msg183824 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-03-09 15:13
Thanks David, the patch looks good. I will commit it with some slight modifications and touch-ups shortly.

The issue of C extension docstrings vs. Python docstrings is an interesting one. It's a shame that help() shows empty strings, and it's a shame to copy-paste. What do other modules do? It may also be worth asking in python-dev or the docs mailing list and hear suggestions from other developers.
msg183825 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-09 15:13
New changeset f27d7c1eac4d by Eli Bendersky in branch 'default':
Issue #16954: Add docstrings for ElementTree
http://hg.python.org/cpython/rev/f27d7c1eac4d
msg186691 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-04-13 03:19
David, would you like to pursue this further (figuring out how to make docstrings show in help() without duplicating?)
msg186948 - (view) Author: David Lam (dlam) * Date: 2013-04-14 19:37
Hi Eli, I sure would!

(Though, if anyone finds this issue and can figure out a solution, I'd encourage them to post it!)
msg186957 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-04-14 21:47
You can ask on the python-dev mailing list. It's possible that other Python developers ran into a similar issue.
History
Date User Action Args
2014-01-09 09:34:36serhiy.storchakalinkissue20159 dependencies
2013-04-14 21:47:17eli.benderskysetmessages: + msg186957
2013-04-14 19:37:37dlamsetmessages: + msg186948
2013-04-13 03:19:56eli.benderskysetmessages: + msg186691
2013-03-09 15:13:43python-devsetnosy: + python-dev
messages: + msg183825
2013-03-09 15:13:10eli.benderskysetmessages: + msg183824
2013-02-28 10:19:55dlamsetfiles: + issue16954-v2.patch

messages: + msg183210
2013-02-20 18:58:57ezio.melottisetmessages: + msg182542
stage: needs patch -> patch review
2013-02-17 05:22:32dlamsetfiles: + issue16954.patch
keywords: + patch
messages: + msg182261
2013-01-24 19:43:15ezio.melottisetmessages: + msg180545
2013-01-24 19:39:27dlamsetmessages: + msg180544
2013-01-24 18:19:14ezio.melottisetmessages: + msg180540
2013-01-24 18:15:11eli.benderskysetmessages: + msg180538
2013-01-24 18:02:10ezio.melottisetmessages: + msg180536
2013-01-18 18:56:34tshepangsetnosy: + tshepang
2013-01-15 10:21:28serhiy.storchakasetassignee: serhiy.storchaka -> docs@python
2013-01-15 04:34:57dlamsetnosy: + dlam
2013-01-14 11:09:56serhiy.storchakasetassignee: docs@python -> serhiy.storchaka
2013-01-14 02:17:44ezio.melottisetnosy: + ezio.melotti
2013-01-13 17:17:03eli.benderskysetkeywords: + easy

messages: + msg179883
2013-01-13 15:26:53serhiy.storchakacreate