Skip to content

Update the documentation guidelines #11495

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

Merged
merged 1 commit into from
Jul 1, 2018

Conversation

timhoffm
Copy link
Member

PR Summary

Pulling some documentation aspects proposed in #10225 into the documentation guidelines.

The guidelines do still need further updates. However, lets start with these so that the amount of changes is still manageable for reviewers.

@fredrik-1
Copy link
Contributor

I agree with those changes. One thing I have though about though and I was about to write about in the discussion. I have no problem if this get merge directly though.

I believe that the matplotlib in for example ~matplotlib.axes.Axescould be left out most of the times. It makes the string longer without given much information for most users.

I believe that it often make sense to include the module name also in the rendered documentation. Some docstrings, like for the colorbar module, looks strange when it is rendered as colorbar is a thin wrapper of colorbar. It would be easer to read as pyplot.colorbar is a thin wrapper of Figure.colorbar.

I believe that I in most cases want to read the module name also in the rendered documentation.

.. code-block:: rst
Sphinx claims to automatically link types in ``Parameters`` and
``Returns`` sections. However, this does currently not work reliably.
Therefore we still have to enclose them in backticks.
Copy link
Contributor

@fredrik-1 fredrik-1 Jun 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true? I believe that the only section where backticks are not necessary is the See Also section.

I think that the whole note can be deleted.

@NelleV
Copy link
Member

NelleV commented Jun 26, 2018

That looks good to me (although I am worried about the length of the document. Those are typically never read by our contributors, including core-contributors)

@timhoffm
Copy link
Member Author

Even if not everyone follows those guidelines exactly, it's still of value. It can be seen as an aspirational goal, and if we follow at least partly we're still converging in that direction. Actually, there have been requests for a better documentation guide. After all, it can help a contributor to find out how to do certain things. We have a lot of different styles in the actual docs, because the guidelines have changed over time. Having an up-to-date document to reference when in doubt is a good thing. This is essentially a collection of proven best practices.

The length can actually be a bit daunting. I've tried to cut back on duplication, give it more structure and add examples, so that one can find out certain aspects without having to read the complete document. It's still far from perfect. There can be done more to clean up the guidelines, but it's a step in the right direction.

@NelleV
Copy link
Member

NelleV commented Jun 26, 2018

@timhoffm I agree with all of what you say :) It's just typically easier when all of the documentation follows the same convention already. That's the challenge with an 18year-old project, that predates most of the modern documentation tools and documentation.

I'd like this to be reviewed by another core developer before the document being merged, as this clarifies some conventions, and we ideally all should be in sync on that.

@NelleV NelleV requested a review from choldgraf June 30, 2018 20:18
@NelleV
Copy link
Member

NelleV commented Jun 30, 2018

If we don't get another review on this soon, I'll just go ahead and merge it.


.. code-block:: python
*Note:* We use the sphinx setting `default_role = 'obj'` so that you don't
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not make these .. note:: so it'll render that way in the site?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather a side note. It's not something you have to know to be able to write documentation. As such a .. note:: admonition would give it too much attention.


.. code-block:: python
*Note:* We use the sphinx setting `default_role = 'obj'` so that you don't
have to use qualifiers like `:class:`, `:func:`, `:meth:` and the likes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On that note, I believe that these need to be double-backticks in rST, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this as well, but I believe we are using the very nice option that makes the documentation build fail if there's any warnings, so I guess these are correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may not give a warning though, it may just not display as in-line code, but as regular text w/ backticks

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. These should be double-backticks. Fixed.

@NelleV
Copy link
Member

NelleV commented Jul 1, 2018

Ok, this looks great. Thanks @choldgraf for the review (despite the bad timing 😄 ) and thanks @timhoffm for updating the docs on writing docs!

@NelleV NelleV merged commit df2c7a9 into matplotlib:master Jul 1, 2018
@timhoffm timhoffm deleted the documenting-mpl branch July 1, 2018 20:09
@timhoffm timhoffm mentioned this pull request Jul 2, 2018
6 tasks
@tacaswell tacaswell added this to the v3.0 milestone Jul 8, 2018
@Kojoley
Copy link
Member

Kojoley commented Jan 7, 2020

Backporting this to v2.2.x to fix CI issues there

@meeseeksdev backport to v2.2.x

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 7, 2020
Kojoley added a commit that referenced this pull request Jan 7, 2020
Backport PR #11495 on branch v2.2.x (Update the documentation guidelines)

Co-authored-by: Nelle Varoquaux <nelle.varoquaux@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants