Adopting a docstring convention

[happz]

In the light of my attempt to enable (some) docstring checks in ruff linter, it would be nice to pick a docstring convention. Such a convention defines how various aspects of the documented object should be expressed in the docstring, e.g. function parameters or what it returns, but also defines (some) formatting requirements, e.g. whether a single-line docstring should indeed be written on a single line, or whether its leading & trailing triple quotes should sit on their own lines. We already accepted some rules, e.g. a single-line string shall occupy a single line, including its triple quotes, and shall not end with a dot, a convention would bring more of them.

Note that this applies purely to docstrings, it's not that visible to users, unless they dive into the development part of docs generated from docstrings.

Cons:

• more rules to follow :(
• yet another linter might go angry on my code :(
• for someone who's not that interested, available conventions might look quite the same.

Pros:

• more rules to follow \o/
• yet another linter to enforce we adhere to the same format \o/
• docstrings should provide documentation for us, tmt developers, and document what various pieces of our code do and why. Having a common format to express these concepts would make it easier to consume them.

Conventions

PEP287 - reStructuredText Docstring Format

Basic "one shall use ReST to write their docstrings" rule. It does not discuss details of how attributes of documented objects would be expressed, only states that ReST is way better than plain text, and it should be adopted as the markup language of choice.

We are already on board, we don't use markdown or asciidoc in our docstrings. The discussion is about what to do beyond this basic setup.

PEP257 - Docstring Conventions

Sets some very, very basic rules - use triple quotes, a blank line here but not there, and so on. I believe we do have the basics covered, although we do not follow all the rules described in this PEP. For example:

There’s no blank line either before or after the docstring.

I for one break this rule quite often as I find the "... or after the docstring" bit making code harder to read. A blank line separating a docstring and the actual code is nice to have in my books, and I've been following this consistently during my career, and there are parts of tmt code I did not touch that do add a blank line after a docstring.

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 …”.

Our single-line docstrings definitely do not end with a period...

---

I believe it's good to read these two PEPs, and be aware of their content, but we do not have to stick to them word by word. Especially the PEP257 which defines some conventions we already break and might be pretty happy to continue doing so in the future. We should be able to configure ruff to enforce our current style as exceptions to these basic rules.

The gist of the discussion is IMO the convention for the description of various object attributes.

Sphinx Python domain

Builds upon PEP287 ("Use ReST") and defines ways to describe various aspects of documented objects, linking between them and other goodies provides by Sphinx.

"""
Parse raw constraint specification into our internal representation.

:param name: name of the constraint.
:param raw_value: raw value of the constraint.
:param as_quantity: if set, value is treated as a quantity containing also unit, and as
    such the raw value is converted to :py:class`pint.Quantity` instance.
:param as_cast: if specified, this callable is used to convert raw value to its final type.
:param original_constraint: when specified, new constraint logically belongs to
    ``original_constraint``, possibly representing one of its aspects.
:param allowed_operators: if specified, only operators on this list are accepted.
:raises ParseError: when parsing fails, or the operator is now allowed.
:returns: a :py:class:`Constraint` representing the given specification.
"""

Google

"""
Parse raw constraint specification into our internal representation.

Args:
    name: name of the constraint.
    raw_value: raw value of the constraint.
    as_quantity: if set, value is treated as a quantity containing also unit, and as
        such the raw value is converted to :py:class`pint.Quantity` instance.
    as_cast: if specified, this callable is used to convert raw value to its final type.
    original_constraint: when specified, new constraint logically belongs to
        ``original_constraint``, possibly representing one of its aspects.
    allowed_operators: if specified, only operators on this list are accepted.


Raises:
    ParseError: when parsing fails, or the operator is now allowed.

Returns:
    a :py:class:`Constraint` representing the given specification.
"""

NumPy

"""
Parse raw constraint specification into our internal representation.

Parameters
----------
name : str
    name of the constraint.
raw_value : any
    raw value of the constraint.
as_quantity : bool
    if set, value is treated as a quantity containing also unit, and as
    such the raw value is converted to :py:class`pint.Quantity` instance.
as_cast : callable
    if specified, this callable is used to convert raw value to its final type.
original_constraint : Constraint
    when specified, new constraint logically belongs to
    ``original_constraint``, possibly representing one of its aspects.
allowed_operators : list[Operator]
    if specified, only operators on this list are accepted.

Raises
------
ParseError: when parsing fails, or the operator is now allowed.

Returns
-------
a :py:class:`Constraint` representing the given specification.
"""

---

As you can see, they are very similar in their use of ReST, but also different in their approach to actual attributes. Some are more verbose, some are denser. NumPy expects us to mention parameter types which is something that can be easily extracted from the signature (there's a Sphinx plugin for it, and we already use it), the other two do not require this.

[martinhoyer]

I don't belive it's worth switching from PEP-based docstrings to Google or Numpy.
While I haven't tried it myself, I assume that setting PEP-257 (none by default) in the linter and then tweak it as needed will be better than selecting and ignoring pydocstyle rules individually.

As for what's in the docstrings, I can't comment on that, as I don't know much about how the documentation is being generated(also I like MkDocs). But would keep it as lean as possible - I mean obvious things like don't write the type as the variables are type annotated, or just writing "Returns true if foo" instead of the whole ":returns:" stuff.

[happz]

I for one prefer a structured approach, i.e. :returns: or Returns with the return value description rather than free-text expression. I can’t say how many times I cursed while reading Python stdlib docs, looking for a parameter description, and I had to search for it in several paragraphs of text… With fields, rendering tools can extract the info and provide it in a structured form, while humans can still write their hearts out in the rest of the docstring :)

[martinhoyer]

Let me rephrase it - my way of thinking is that human should only spend time on description, useful examples and generally things that would be hard/impossible to generate reliably with modern tooling.

[happz]

I see, you probably speak about :rtype: which is a return value type, and that can be extracted from signature while :returns: is supposed to be a description of the returned value, describing it to humans.

[LecrisUT]

Isn't sphinx domain the best supported format for PyCharm? I think there was some issue with duplicated documentation coming from the auto-generated one as well.

Also sphinx style uses """ format for attribute documentation (docstring after class's attribute definition) while the others technically don't have one and they are all in the class's docstring. There are a few other available conventions, but that's probably the most intuitive.

[martinhoyer]

Isn't sphinx domain the best supported format for PyCharm?

Epytext and reStructuredText.

Not sure about their new IDE.

[happz]

Isn't Sphinx using #: foo comments before an attribute for documentation?

[happz]

Isn't Sphinx using #: foo comments before an attribute for documentation?

Yes. Both:

For module data members and class attributes, documentation can either be put into a comment with special formatting (using a #: to start the comment instead of just #), or in a docstring after the definition.

I've been using #: to document individual module-level names and class attributes too, as I'm used to it, I suppose it wouldn't be a problem to convert them to strings. I'm skeptical about documenting class attributes in class docstring though, it detaches documentation from the attribute and suddenly it's easier to add an attribute and not document it. But maybe ruff can catch that.

[thrix]

If I have to vote, I am a Sphinx Python domain fan

[happz]

There's a poll above :)

[psss]

Thanks much for the great overview, @happz! While we're speaking about the docstring conventions I'd like to suggest these two as well:

• Use infinitive form when describing what's happening, e.g. Prepare the special variable instead of Preparing... or Prepares..., just a nitpick, but it's the shortest form and nice to have things consistent. It would also make sense for commit summaries.
• I'd recommend adhering to the maximum recommended line of 72 characters for the documentation strings and comments, I believe it's an optimal width for reading longer text.

The first one could go to recommendations, the seconds could be covered by automated checks.

[LecrisUT]

The second can be covered by ruff-format (tool.ruff.format.docstring-code-line-length). Not sure about other tools, or if we should be considering them, given how many of the format/lint are moving to ruff.

[martinhoyer]

* Use infinitive form when describing what's happening, e.g. `Prepare the special variable` instead of `Preparing...` or `Prepares...`, just a nitpick, but it's the shortest form and nice to have things consistent. It would also make sense for commit summaries.

I guess that can be covered by D401, which is enabled by default with PEP convention.

* I'd recommend adhering to the maximum recommended line of 72 characters for the documentation strings and comments, I believe it's an optimal width for reading longer text.

On linter, I belive we already have this in pycodestyle (W505), but I think it should mainly be set in formatter. Ruff formatter have a setting for it, while autopep8 I guess is by default 72.

[LecrisUT]

I misread docstring-code-line-length, that one is formatting the code examples, not the whole docstring. tool.ruff.lint.pycodestyle.max-doc-length seems to be the relevant one. Not sure if ruff-format takes that into account then, but probably yes.

[martinhoyer]

@LecrisUT yeah ruff formatter takes linter rules into account and vice cersa :)

[LecrisUT]

Wanted to share a tool that I've found that works with sphinx-style attribute docstrings (as well as all other formats): https://github.com/rr-/docstring_parser

Might be interesting to incorporate dataclasses/attrs with it 🤔

Edit: Welp, I have been on a long rabbit-hole, basically it is currently impossible to attach docstrings to class attributes. The only "clean" way I've found was to add a doc field to the attrs.Attributes and then introspect the attribute's docstring using attrs.fields