-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add guidelines for writing inline docstrings to the manual #15136
Conversation
Related: #8966 This PR is more restricted in its scope, as it merely attempts to standardize existing practices. But the part about the argument lists section could certainly be extended to list other sections in the future. |
purpose. An argument list would only repeat information already provided elsewhere. | ||
However, it it might be a good idea for complex functions with many arguments. In | ||
that case, put the argument list after the general description of the function, under | ||
a ``# Arguments:`` header, with one ``*`` bullet for each argument. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, it makes more sense to use ### Arguments:
, since higher-level headings are too heavy. But it doesn't make much sense IMHO that we would need to use third-level headings for sections, which are arguably top-level for a docstring. So maybe top-level headings should be presented in a less heavy way (i.e. with a single underline)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, rust uses that style (https://doc.rust-lang.org/book/documentation.html#special-sections), and it reads quite nicely I think. Maybe dispense with the :
at the end of the title?
We'd need to do a little bit of re-styling of the terminal output, as you mention, and the style for generated docs, but that's not much of a roadblock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, you're fast! So let's keep it as #
and expect the display to change. The important point is to have some consistency in the definition of sections.
I agree the :
can be left out, will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had written something before (in Lexicon IIRC) which normalized the headers in a block e.g. all be at most h3. If there's an h1, all get 2 subtracted etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be a good solution indeed. At least it would avoid breaking existing documentation.
Markdown happens to be the default, but one can construct other string | ||
macros and pass them to the ``@doc`` macro just as well. | ||
1. Always show the signature of a function at the top of the documentation, | ||
with a four-space ident so that it is printed as Julia code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ident" -> "indent"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I always make this mistake. Will fix.
|
||
@bar buy_drink_for("Jiahao") | ||
Example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this should be another section, will change to # Examples
.
Rust also has a RFC with detailed guidelines like the ones I wrote here: https://github.com/rust-lang/rfcs/blob/master/text/0505-api-comment-conventions.md |
that case, put the argument list after the general description of the function, under | ||
a ``# Arguments`` header, with one ``*`` bullet for each argument. | ||
|
||
4. When adding examples, use ``jldoctest`` blocks instead of standard text. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is jldoctest
? How do I run doctests? Needs some explanation, maybe combined with something in the Unit Tests section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I need somebody to tell me whether/how to run doctests. Even if it's not yet possible, it would still be a good idea to advise people to use them so that everything is ready when support lands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment this is base only cc @MichaelHatherly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, base only. I believe they are run with sphinx. I'd like to eventually just have any julia
code block get tested without having to manually add a doctest flag to code blocks, though that might be too aggressive perhaps. Checking whether the block contains any REPL prompts, i.e. julia>
, and running those may be a suitable heuristic. There's some work done in this direction in Lapidary.jl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So should I advise using julia
blocks with julia>
prompts for now (and only indicate that in the future it will be used for automated testing)?
Thanks, look like reasonable suggestions to me. |
@nalimilan Is it worth putting something about line length in (ie. breaking up large paragraphs into lines of 80 chars)? Seems that it is a common habit to put in one long line. |
Indeed, makes sense. |
What is the recommended line length with docstrings? CONTRIBUTING.md recommends a 92 character limit for code. |
I'd say we should follow the same rule, since docstrings are mixed with code and edited using the same tools. |
makes sense! I missed the 92 char limit. Anyone know significance of 92? |
This allows running examples automatically and checking that their actual output is | ||
consistent with that presented in the documentation. This way, the code is tested and | ||
examples cannot get out of date without notice. Examples should be grouped under an | ||
``# Examples`` section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought base / packages were using ###
or ####
, IMO h1 is very high.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See now hidden discussion at #15136 (comment)
H1 is high, but within a docstring I don't see what could be higher than a section title...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
it's true, IMO the issue comes if you use all of h1-6, but that's an edge case that can be warned for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 levels for a docstrings would be a lot. My idea was rather to avoid the noise of using three # when one would be enough, as well as the oddity of starting headings at level 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
H1 is high, but within a docstring I don't see what could be higher than a section title...
Yes, the docstring should be able to act as a standalone "document", so each major section of the "document" is in fact a toplevel section regardless of whether it might be embedded in some other larger document at some point.
The noise argument is also a compelling one for a single #
.
Also possibly worth mentioning whether to use the plural # Examples
even if there's only one example. I'd be in favour of sticking to always using plural form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a serious formatting incompatibility with Sphinx in allowing section headings in docstrings, at least for Base
. We currently put extracted docstrings back into the stdlib docs where they go under function blocks, but Sphinx complains with:
SEVERE: Unexpected section title.
the moment you try to use any section headings whatsoever in a docstring that gets spliced into the docs.
What's the fix for this problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess they could be turned into standard bold font as a workaround.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a bit unfortunate. Post-processing in genstdlib.jl
to convert headers to bold might be a workable solution for the moment.
When I did the last rst->md conversion that's the style I stuck to. Probably best to just have a single line limit for both docs and code rather than having two different ones. Some other thoughts that could be relevant: Formatting: Give docstrings ample spacing to aid readability since vertical space isn't at a premium. Place the starting and ending """
...
...
"""
func(x, y) = ... rather than """...
..."""
func(x, y) = ... to make it more clear where docstrings start and end. Documenting functions/methods: When a module extends a generic function from another module avoid adding new docs for every Prefer documenting which functions a type extends in the type's docstring. So """
...
# Extends
- `Other.func`
- ...
...
"""
type T end
Other.func(::T) = ... rather than """
...
...
"""
type T end
"""
Calls `func` on a `T`...
"""
Other.func(::T) = ... (Not sure how well this approach would work in practice though, or whether it would be any better.) Summary Line: After the simplified signature block include a single one line sentence describing what the See for example the docstring for """
push!(collection, items...) -> collection
Insert one or more `items` at the end of `collection`.
...
"""
... LaTeX syntax: Perhaps worth mentioning double backtick syntax and """
... ``α = 1`` ...
""" rather than """
... ``\\alpha = 1`` ...
""" |
Is auto-populating signatures, at least for simple cases, still on the radar somehow? |
|
||
@bar buy_drink_for("Jiahao") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sorry, I wanted a more complex example with optional arguments and a detailed description of the parameters. Maybe add an optional vector of beer types?
More seriously, I realize it would make sense to add the types to the signature, since that's what it advised below.
@MichaelHatherly Good points, I'll add these. Though about extending methods, the PR already mentions that one shouldn't repeat the function's docstring. I'm not sure about listing extended methods in the type's documentation: that's quite verbose, and |
Yup, sure. I'm not terribly attached to the idea. What you've added about not repeating docs should be good enough for now. Thanks.
Yes, it is. Hopefully before 0.5 feature freeze... whenever that is. |
Regarding headings and extending methods, I notice that at the moment, Julia's help system will display the docstrings for all methods of a function. So if you enter
you get the docstrings for multiplication, string concatenation, and matrix multiplication, one after the other, and the call signatures are the only thing delimiting them. If there is other Julia code in the docstring, then the call signatures don't stand out at all. The H1 level seems like a natural choice to delimit different methods' docstrings. How about recommending use of the call signature with the H1 level as the first line of a docstring? Quick example at the Julia prompt:
(Pretty cyan text for code highlighting the call signatures not shown here.) The other nice feature is this still looks good if you get help for a specific method:
I think it also looks quite good in an IJulia notebook. I agree that in many or most cases a separate docstring for each signature is unwarranted, but for those cases in which it is good to have, this seems like a good solution to me. |
@biogeo I think this would be better handled directly by the printing method. For example, it could add some line breaks, or a horizontal separator, to make different methods visually distinct. That way, we don't add visual noise to functions with only one docstring. |
I've just pushed an updated commit which should take into account all remarks. Please tell me what you think. |
d1c319b
to
943c508
Compare
function bar(x, y) ... | ||
|
||
Documentation is very free-form, though we recommend following some simple | ||
conventions, illustrated in the above example. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentence could use a little rework. Maybe:
Documentation, as illustrated above, is very free-form though we recommend following some simple conventions:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I've even removed the mention of "free-form", since the whole point of this section is now to give guidelines.
Nice work @nalimilan. |
@nalimilan Thank you for considering it. I'll send a pull request if I find inconsistency. |
Any more objections/ideas, or is it OK to merge? |
👍 |
I'll merge soon if nobody objects. I guess this also needs backporting to the 0.4 manual. |
(like ``mean(x::AbstractArray)``), or a simplified form. | ||
Optional arguments should be represented with their default values (i.e. ``f(x, y=1)``) | ||
when possible, following the actual Julia syntax. Optional arguments which | ||
do not have a default value should be put brackets (i.e. ``f(x[, y])`` and ``f(x[, y[, z]])``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put in brackets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I've just fixed it (I also changed <arguments>
to <keyword arguments>
).
Also add comments to prevent them from getting out of sync again.
See #15627 for backport. |
Does anybody know how to get
to render correctly in Sphinx docs? The double backslash breaks everything. |
|
Unfortunately not, this gives three backslashes, and the backquotes are still outside the block. (To see what it looks like currently, look for |
@nalimilan in the RST files you need to do
edit: I just realized you don't want to render the actual alpha symbol... |
I can't get it to render as an inline code block. It seems like the only way to get that to render correctly is to use a code block indented by four-spaces. |
@nalimilan there is probably a better RST way but this does work:
renders inline as
|
Thanks! I've added this fix to #15627 and I'll push it to master directly after the tests pass. |
Now that we have some experience with writing docstrings, it looks like a good time to try providing some guidance about how to organize them (a question which emerged at JuliaCollections/DataStructures.jl#182). So here's a proposal for discussion.