Specifics of docstrings style

[klee]

Some details of the Sage docstring styles are not clearly stated in the developer manual, and left to each writer's discretion. This introduced inconsistencies and sometimes allowed bad styles. The purpose of this ticket is to spot those details and provide specific guidelines for them, based on the consensus of the developer community.

The guidelines collected here will be used to revise

​http://doc.sagemath.org/html/en/developer/coding_basics.html#documentation-strings

---

H1. Write

``True``, ``False``, ``None``

but do not write

True, False, None

This was discussed at ​https://groups.google.com/forum/#!topic/sage-devel/EVG7wQExYOQ

---

H2. It is preferable to write

if the lattice is reflexive

than

if ``self`` is reflexive

This was discussed at ​https://groups.google.com/forum/#!topic/sage-devel/dm8PQbni0Y8

---

H3. Write

Return whether this lattice is reflexive.

but do not write

Return ``True`` if the lattice is reflexive and ``False`` otherwise.

This was discussed at ​https://groups.google.com/forum/#!topic/sage-devel/OFuYydUMMzI

---

H4. OUTPUT block is optional but recommended unless it is clear from the one-line explanation.

This was discussed at ​https://groups.google.com/forum/#!topic/sage-devel/i9pemQvEE5E

---

H5. Write

OUTPUT: a tuple of lattices

This was discussed at ​https://groups.google.com/forum/#!topic/sage-devel/bdLBvSlJLPo

---

H6. Write

INPUT: 

- ``n`` -- integer (default: 1); the number of repetition

This was discussed at ​https://groups.google.com/forum/#!topic/sage-devel/eMkeLj_w9ZY

[jdemeyer]

I would like to add that INPUT and OUTPUT blocks should be sufficiently verbose.

Some places in Sage have something like

INPUT:

- ``x`` -- integer

Well, that's not very useful. If should explain the purpose of x, not just the fact that it's an integer.

[jdemeyer]

It might also be good to formally define the style of types and defaults in INPUT blocks. I usually use

INPUT:

- ``p`` -- (integer, default: `11`) the modulus for the modular algorithm

[tscrim]

Strong -1 to G1. True and similar should be code because they are Python objects.

Strong -1 to G2. self is a well-defined concept in Python, more concise, and more clear when subclasses are involved.

Subsequently, I see no good options in G3. However, option 3 is closet, and I prefer:

Return whether ``self`` is reflexive.

G4: recommended but optional. This is what we primarily do in practice anyways.

+1 on comment:5, although I do a slightly different style

INPUT:

- ``p`` -- integer (default: `11`); the modulus for the modular algorithm

[mforets]

have you considered here the guidelines for writing an exception string?

no beginning upper case, no final full stop is what i've been told, but i just quickly checked ​documentation strings chapter and there are seemingly no guidelines.

only: “It is an error to ..”

thanks

[klee]

Replying to mforets:

have you considered here the guidelines for writing an exception string?

No. This ticket is about docstrings.

If you have specific ideas, you may start your own ticket and polls about exception strings. But do you really have specific ideas to guide developers on exception strings other than "no beginning upper case, no final full stop"? Hmm, I don't have...

[mforets]

Replying to klee:

Replying to mforets:

have you considered here the guidelines for writing an exception string?

No. This ticket is about docstrings.

If you have specific ideas, you may start your own ticket and polls about exception strings. But do you really have specific ideas to guide developers on exception strings other than "no beginning upper case, no final full stop"? Hmm, I don't have...

sure. the point was that "no beginning upper case, no final full stop" was sth that i learnt here in trac, which is ok, but better if it is documented somewhere close to the Guide (if i didn't miss it).

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​18ebc5e

Add some specific dosctring guides

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​92812ec

This is the final one

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​21fcb16

Modify the template

[dkrenn]

1)

+       The lexicographic product of G and H is the graph with vertex set ...

use ... of `G` and `H` (or ... of ``G`` and ``H``) for correct typesetting here. (I prefer `G`.)

2)

+       - ``p`` -- prime integer (default: 2); coprime with ``n``

use (default: `2`) (or (default: ``2``)) for correct typesetting. (I prefer `2`.)

Change this in the template (lines +540,28) as well.

3)

+       OUTPUT: a tuple of

shouldn't this be a tuple ``(H, U)`` of ?

4) More a question, as I am still compiling my current SageMath: Is the link in

+processed by Sphinx. See http://www.sphinx-doc.org/rest.html for an

typeset correctly?

I did not review the bibliography-part carefully, but were there any changes or was it simply changeing indent?

[dkrenn]

5)

+- Lines should be shorter than 80 characters.

Here one could link to the PEP8 ​https://www.python.org/dev/peps/pep-0008/#maximum-line-length

[klee]

Replying to dkrenn:

I did not review the bibliography-part carefully, but were there any changes or was it simply changeing indent?

Just changing the indent.

[klee]

Replying to dkrenn:

4) More a question, as I am still compiling my current SageMath: Is the link in

+processed by Sphinx. See http://www.sphinx-doc.org/rest.html for an

typeset correctly?

Yes.

[jmantysalo]

Template has a .. NOTE:: block of what should be an ALGORITHM:. Not sure if that should be changed in this ticket or not.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​d2c8676

Minor fixes

[klee]

Replying to dkrenn:

1)

+       The lexicographic product of G and H is the graph with vertex set ...

use ... of `G` and `H` (or ... of ``G`` and ``H``) for correct typesetting here. (I prefer `G`.)

Done.

2)

+       - ``p`` -- prime integer (default: 2); coprime with ``n``

use (default: `2`) (or (default: ``2``)) for correct typesetting. (I prefer `2`.)

Change this in the template (lines +540,28) as well.

Done.

3)

+       OUTPUT: a tuple of

shouldn't this be a tuple ``(H, U)`` of ?

I thinks "a tuple of" suffices here.

[klee]

Replying to jmantysalo:

Template has a .. NOTE:: block of what should be an ALGORITHM:. Not sure if that should be changed in this ticket or not.

This ticket will not touch that. I think that block is close to a note as it does not explain the algorithm itself.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​2e0d0f8

Add PEP8

[jdemeyer]

If the OUTPUT block is "optional but recommended", shouldn't the same be true for the INPUT block? I am thinking of this docstring for example:

cpdef edge_connectivity(g):
    r"""
    Computes the edge connectivity of the input graph, using Boost.

    OUTPUT: a pair ``(ec, edges)``, where ``ec`` is the edge
    connectivity, ``edges`` is the list of edges in a minimum cut.

Here, there is no explicit INPUT but I don't think it's needed anyway.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​3325c92

INPUT block is not "not optional"

[klee]

Replying to jdemeyer:

If the OUTPUT block is "optional but recommended", shouldn't the same be true for the INPUT block?

Certainly there are many cases that INPUT block is not needed, and in practice we do not add one in such cases. So not optional is too much prohibitive. I just removed the sentence.

[jhpalmieri]

If you want to follow the guidelines here (all Sphinx directives are written in uppercase), then you should change ".. code-block::" to ".. CODE-BLOCK::". You should also change it from ".. code-block:: none" to ".. CODE-BLOCK:: rest".

The bulleted point starting with "Lines should be shorter than 80 characters." has a line which is exactly 80 characters, so perhaps that should be broken up so all lines are shorter than 80 characters. Some of the other lines (for example in the paragraph starting "The decomposition of the free module ..." are now longer than 80 characters.

Why did you delete "If it contains multiple lines, all the lines after the first need to begin at the same indentation as the backtick"? I think that is helpful information, and people often make mistakes with this indentation in docstrings.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​c3aaf3b	Merge branch 'develop' into docstring_trac23017
​00253ce	Fixes to follow the guidelines

[klee]

Replying to jhpalmieri:

If you want to follow the guidelines here (all Sphinx directives are written in uppercase), then you should change ".. code-block::" to ".. CODE-BLOCK::". You should also change it from ".. code-block:: none" to ".. CODE-BLOCK:: rest".

Done.

The bulleted point starting with "Lines should be shorter than 80 characters." has a line which is exactly 80 characters, so perhaps that should be broken up so all lines are shorter than 80 characters. Some of the other lines (for example in the paragraph starting "The decomposition of the free module ..." are now longer than 80 characters.

I broke up many lines to make it shorten than 80 characters. But I kept some of the lines as they look better as they are than broken up. This includes the second line of the bulleted point, which has exactly 80 characters and the 80th character is a period. I think this is a good example showing that a guideline is just a guideline and not to be followed blindly when there are good reasons not to follow :-)

Why did you delete "If it contains multiple lines, all the lines after the first need to begin at the same indentation as the backtick"? I think that is helpful information, and people often make mistakes with this indentation in docstrings.

Recovered.

[jhpalmieri]

I would suggest one more change:

src/doc/en/developer/coding_basics.rst

@@ -507 +507 @@
 - All reST and Sphinx directives (like ``.. WARNING::``, ``.. NOTE::``,
   ``.. MATH::``, etc.) are written in uppercase.
 
-- Code fragments are quoted with ``````. This includes function arguments
-  and the Python literals like ````True````, ````False```` and ````None````.
+- Code fragments are quoted with double backticks. This includes
+  function arguments and the Python literals like ````True````,
+  ````False```` and ````None````. For example:
+
+  .. CODE-BLOCK:: rest
+
+      If ``check`` is ``True``, then ...
 
 Sage's master **BIBLIOGRAPHY** file
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Otherwise, it is okay with me.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​f752318

Add example for code fragments

[jhpalmieri]

Okay, thank you.

[dkrenn]

Fine for me as well (it seems that I was removed from the reviewers field; I've added my name again; all my suggestions above were incoperated.)

[klee]

Thank you!