Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Update documentation guide#9805
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
doc/devel/documenting_mpl.rst Outdated
| .. rubric:: Footnotes | ||
| .. [#] For example. | ||
| * Use the *note* and *warning* directives, sparingly, to draw attention to |
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.
This delted block is covered by numpydoc guidelines
| The function `matplotlib.artist.kwdoc` and the decorator | ||
| `matplotlib.docstring.dedent_interpd` facilitate this. They combinepython | ||
| `matplotlib.docstring.dedent_interpd` facilitate this. They combinePython |
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.
Given the big squak some raised when I tried to use this facilty, should this be in the doc guide?
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'll still re-iterate that I was told explcitlynot to do this for the legend docs a month ago. So if we are not allowed to use this functionality it should be noted here.
doc/devel/documenting_mpl.rst Outdated
| @@ -91,244 +90,131 @@ statement, such as:: | |||
| .. include:: ../../TODO | |||
| docstrings | |||
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.
This whole section has moved down to the bottom
doc/devel/documenting_mpl.rst Outdated
| directories. To exclude the example from having an image rendered, | ||
| insert the following special comment anywhere in the script: | ||
| automatically included in the HTML docs. To exclude the example from having an | ||
| image rendered, insert the following special comment anywhere in the script: |
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.
Is this still true@choldgraf ?
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 no, instead they should putsgskip somewhere in the filename. Maybe worth just noting "all files in theexamples directory will be automatically run and rendered into documentation with sphinx-gallery (incl. link)" and leave it at that?
| Please do not describe ``argument`` like this. | ||
| * Mathematical expressions can be rendered as png images in html, and in the |
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've removed the maths, ipython, and footnotes bits, as they are covered in Sphinx documentation, and I don't think they're common enough to need a specific section in the Matplotlib docs.
| The documentation sources are found in the :file:`doc/` directory in | ||
| the trunk. To build the users guide in html format, cd into | ||
| :file:`doc/` and do: | ||
| The documentation sources are found in the :file:`doc/` directory in the trunk. |
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.
This part will be obsoleted byhttps://github.com/matplotlib/matplotlib/pull/9513/files...
doc/devel/documenting_mpl.rst Outdated
| def hlines(self, y, xmin, xmax, colors='k', linestyles='solid', | ||
| label='', **kwargs): | ||
| """ | ||
| Plot horizontal lines at each `y` from `xmin` to `xmax`. |
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.
should be asterisks :p
doc/devel/documenting_mpl.rst Outdated
| * Use the *seealso* directive, for example: | ||
| Other Parameters | ||
| ---------------- | ||
| **kwargs : `~matplotlib.collections.LineCollection` properties. |
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.
one space too many after the colon
doc/devel/documenting_mpl.rst Outdated
| * Please keep the :ref:`glossary` in mind when writing documentation. You can | ||
| create a references to a term in the glossary with the ``:term:`` role. | ||
| Please do not describe `argument` like this. |
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 think a (short) explanation similar to the one I gave at#9786 (comment) (about the meaning of single backticks) may be helpful.
doc/devel/documenting_mpl.rst Outdated
| it via a relative path from the document where the rst file resides, e.g., in | ||
| :file:`users/navigation_toolbar.rst`, we refer to the image icons with:: | ||
| In the documentation, you may want to include to a document in the | ||
| Matplotlib src, e.g., a license file or an image file from `mpl-data`, |
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.
sources
doc/devel/documenting_mpl.rst Outdated
| mpl-data directory, I use the symlink directory. For example, from | ||
| :file:`customizing.rst`:: | ||
| In the `users` subdirectory, if you want to refer to a file in the mpl-data | ||
| directory, you can use the symlink directory. For example, from |
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 don't think there's a symlink anymore?
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.
Line 1 in7ddc73a
| ../examples |
In master it looks like there still is
doc/devel/documenting_mpl.rst Outdated
| @@ -454,7 +287,7 @@ files are duplicated and large) | |||
| Internal section references | |||
| =========================== | |||
| To maximize internal consistency in sectionlabeling and references, | |||
| To maximize internal consistency in sectionlabelling and references, | |||
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.
US english one ell? :)
doc/devel/documenting_mpl.rst Outdated
| @@ -467,8 +300,8 @@ and refer to it using the standard reference syntax: | |||
| See :ref:`howto-webapp` | |||
| Keep in mind that we may want to reorganize the contents later, so | |||
| let's avoid top level names in references like ``user`` or ``devel`` | |||
| Keep in mind that we may want to re-organize the contents later, so | |||
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.
doc/devel/documenting_mpl.rst Outdated
| autoscaled; default True. See Axes.autoscale_view for more | ||
| information | ||
| """ | ||
| pass |
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.
the pass is unneeded (syntax is correct without it and it's not the real impl either)
doc/devel/documenting_mpl.rst Outdated
| The kwargs are Line2D properties: | ||
| %(Line2D)s | ||
| kwargs scalex and scaley, if defined, are passed on |
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.
should use correct markup (asterisks, backquotes below)
doc/devel/documenting_mpl.rst Outdated
| Developer's tips for documenting Matplotlib | ||
| =========================================== | ||
| ================================ | ||
| Developer's tips for documenting |
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.
"Documentation guidelines"?
| the Sphinx_ documentation generation tool. There are several extra | ||
| requirements that are needed to build the documentation. They are listed in | ||
| :file:`doc-requirements.txt` as well as listed below: | ||
| the Sphinx_ documentation generation tool. There are several extra requirements |
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.
also edited in the Makefile PR
doc/devel/documenting_mpl.rst Outdated
| generated directories :file:`docs/gallery` and :file:`docs/tutorials` can be | ||
| safely deleted. | ||
| the directories :file:`docs/gallery` and :file:`docs/tutorials` | ||
| are generated. |
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'd say something like: "...are automatically generated. Do not edit the rst files in :file:docs/gallery and :file:docs/tutorials, as they are rebuilt from the sources in the root directory"
I was mystified for a while when my changes to files in these directories were being overwritten ;-)
doc/devel/documenting_mpl.rst Outdated
| Matplotlib makes extensive use of keyword arguments as pass-through | ||
| arguments, there are a many cases where a table is used in place of a | ||
| definition list for autogenerated sections of docstrings. | ||
| Please do not describe ``argument`` like this. | ||
| Figures | ||
| ======= | ||
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.
This could use an intoroductory paragraph, or even a change of heading. I find it very mysterious what figures you are talking about. In docstrings? I think you mean in examples and tutorials. If so, maybe a "Writing an example or tutorial" would be a more useful heading, and figure creation is of course the most important thing that happens in that context.
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.
Added a bit more text and changed the title; is it better now?
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 think maybe a specific section on writing an example/tutorial is needed.
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.
Maybe a new document? This one is quite long already, so sticking to just "Documenting code" is better, and a separate "Writing examples and tutorials" guide can be written.
| Plot directive documentation | ||
| '''''''''''''''''''''''''''' | ||
| .. automodule:: matplotlib.sphinxext.plot_directive | ||
| :no-undoc-members: | ||
| Examples |
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.
As above. This seems the heading (+ Tutorials) not "Figures" per se. (I guess there are figures in the numpydocs?)
doc/devel/documenting_mpl.rst Outdated
| @@ -492,11 +308,91 @@ For everything but top level chapters, please use ``Upper lower`` for | |||
| section titles, e.g., ``Possible hangups`` rather than ``Possible | |||
| Hangups`` | |||
| Setters, getters, and keyword arguments | |||
| ======================================= | |||
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.
Why is this way down here instead of up w/ the numpydoc 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.
Moved back up
dstansby commentedNov 22, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Okay, I think I'm ready for reviews! (ping@choldgraf) |
Add link to numpydocAdd missing dashFix buildGet rid of extra bitsMove setter/getter/kwarg section to near bottomMore cleanupRemove sections covered in sphinx docsMore small fixesCorrect sgskipUpdate titleMore cleaningMove setters/getters/kwargs backClarify examples/tutorials directoriesFix code indentationImprove figure discussionImprove some headingsAdd intersphinx section
So save others from a few clicks: |
| `~matplotlib.collections.LineCollection` | ||
| It is also possible to add links to code in Python, Numpy, Scipy, or Pandas. |
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.
for example:
.. code-block:: python
numpy.mean
| argument the method accepts. e.g., in `.Line2D`: | ||
| the `~.matplotlib.artist.Artist` class. The setter methods use the docstring | ||
| with the ACCEPTS token to indicate the type of argument the method accepts. | ||
| e.g., in `.Line2D`: |
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.
OK, I've written some docs, and have used matplotlib for a while. I don't know what the first sentence of this section means. Can we clarify for the somewhat uninitiated?
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.
Something like
Artist properties are implemented using setter and getter methods (because Matplotlib predates the introductions of the
propertydecorator in Python). By convention, these setters and getters are namedset_PROPERTYNAMEandget_PROPERTYNAME; the list of properties thusly defined on an artist and their values can be listed by the~.pyplot.setpand~.pyplot.getpfunctions.
Property setter methods should indicate the values they accept using a (legacy) special block in the docstring, starting with
ACCEPTS, as follows: (code example follows)
The ACCEPTS block is used to render a table of all properties and their acceptable values in the docs; it can also be displayed using, e.g.,
plt.setp(Line2D)(all properties) orplt.setp(Line2D, 'linestyle')(just one property).
looks good?
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 that makes sense to me. Thanks!
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.
@dstansby can you apply the patch? or I can do it too
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.
Feel free to push any changes to this section - I don't have the bandwidth to get my head around it at the moment!
| Here is a description of *argument* | ||
| Please do not use the ```default role```: | ||
| Adding figures |
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 like the text below. I'm still not sure about the heading....
General comment: I still think this could use a bit more basic guidance on the different types of documentation at the top. i.e.:
|
My 2 cents: I think this should be merged. Even if it is not perfect it is a definite improvement in the documentation, and we've already iterated on this quite a lot. People can always open less ambitious PRs to spot-check things but@dstansby has already done a lot on this PR. Obviously it's their call if they want to keep iterating on comments etc, but let's not let perfect become the enemy of good. |
I'm tempted to follow@choldgraf's advice and merge this, but it looks like@anntzer and/or@jklymak might want to push some quick changes first. Do you? |
jklymak commentedNov 28, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think its easier for me to work from master, so I'm happy to put a PR in after this improvement! 👍 |
actually a bit busy today, can anyone prepare a PR from my patch? |
Yes, I'll definitely incude your patch. Its very helpful... |
There seem to be a conflict, please backport manually |
Thanks for merging! I was happy to add more changes, but didn't/don't have the time to around now anyway. |
Update documentation guide
Backport pull request#9805 to v2.1.0-doc
Back port taken care of my55830e5 merge of v2.1.0-doc in to 2.1.x |
Uh oh!
There was an error while loading.Please reload this page.
Needs a bit more work, but the aim is tofix#9786 eventually and generally clean the writing documentation guide. Comments very welcome and encouraged! (please also add to the TODO list below)
TODO: