Skip to content
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

Use autosummary in the docs #22589

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

asmeurer
Copy link
Member

@asmeurer asmeurer commented Dec 3, 2021

References to other Issues or PRs

This is a redux of the work that was started in #18594.

Brief description of what is fixed or changed

Our documentation poll is still ongoing, but one thing that is already becoming clear from some of the responses is that our documentation would benefit from splitting the documentation of each function into separate pages. This is achieved with the autosummary Sphinx extension.

Not only does this make each page of the documentation more manageable, but it also makes it easier for Google search results to return the exact function in question, rather than just a large page that contains the function somewhere on it.

For now, I have only modified the matrices.rst file. Please review what this looks like, both in the RST and the generated HTML. If we like how this looks, we can add it to other pages.

Other comments

  • You have to manually add :toctree: ref to each autosummary to tell it where
    to put the autogenerated files. You should always use the name "ref" for
    this for consistency, but you can use something like :toctree: ../ref to put
    it on level up. The word "ref" is part in the URL for each generated page.

  • The classes I have documented here really need to be organized in a better
    way. However, I have not done that in this PR. An advantage of autosummary
    is that we can move where a class is documented in the organization without
    breaking the URL to it, since it lives on a separate page.

  • This will break any existing links to specific anchors on this page.

  • The class template hardcodes :members:. However, it would be easy to use
    other combinations by adding additional templates, although each autosummary
    list can only use one template at a time.

  • The docs for classes still list each method on the same page. This is
    because autosummary is not really capable of doing recursive autosummary on
    class members in a way that only includes the members we want to include.
    See the commit message of a0e955b.

  • Autosummary is very particular about how you spell the name of the function
    so that it can find it. I recommend avoiding .. module:: or .. currentmodule::and just spelling everything explicitly, using ~ before the
    name to truncate the module name for public names (eventually, we will have
    only public names in this part of documentation, see
    Use autoapi for the internal reference documentation #22105, but this is a separate issue to
    clean this up).

  • When building, be aware that files for the autosummary are generated
    automatically, but are only cleaned when you do a make clean. Thus if you
    add something to an autosummary and then delete it, you will get warnings in
    the build from the generated file that no longer is referenced anywhere.

Release Notes

  • other
    • Start using the autosummary extension in the docs, which puts documentation for each function and class on a separate page.

Screenshots

Here is what the matrices page looks like
Screen Shot 2021-12-02 at 7 38 34 PM

and here is what a page for one of the clases looks like

Screen Shot 2021-12-02 at 7 38 59 PM

asmeurer and others added 4 commits December 2, 2021 15:19
Based on sympy#18594.

Co-authored-by: S.Y. Lee <sylee957@gmail.com>
The name is part of the URL, so it should be something legible. "ref" is
better than "_generated". Personally, I would prefer if it didn't add anything
at all to the slug, but this seems quite hard to do.
We unfortunately cannot use the default template, which recursively uses
autosummary on all the methods of the class, because this just documents every
method of the class, including methods inherited from Basic, private "dunder"
methods, and methods with no docstrings, and there is no way to configure
this. It can't even be configured in the template because the autosummary
template variables don't know about the autodoc distinctions for things like
inherited-members.

One option in the future may be to use autodocsumm instead of autosummary,
which does know about the various autodoc distinctions. Unfortunately,
autodocsumm does not support putting the documentation on separate pages,
which is the whole point of our using autosummary in the first place. See
Chilipp/autodocsumm#66.
This serves primarily as a demonstration of autosummary. If we like the way
this looks, in both the RST and final HTML, we can look at using it
everywhere.

Some notes here:

- You have to manually add :toctree: ref to each autosummary to tell it where
  to put the autogenerated files. You should always use the name "ref" for
  this for consistency, but you can use something like :toctree: ../ref to put
  it on level up.

- The classes I have documented here really need to be organized in a better
  way. However, I have not done that in this PR. An advantage of autosummary
  is that we can move where a class is documented in the organization without
  breaking the URL to it, since it lives on a separate page.

- This will break any existing links to specific anchors on this page.

- The class template hardcodes :members:. However, it would be easy to use
  other combinations by adding additional templates, although each autosummary
  list can only use one template at a time.

- The docs for classes still list each method on the same page. This is
  because autosummary is not really capable of doing recursive autosummary on
  class members in a way that only includes the members we want to include.
  See the commit message of the previous commit.

- Autosummary is very particular about how you spell the name of the function
  so that it can find it. I recommend avoiding `.. module::` or `..
  currentmodule::` and just spelling everything explicitly, using `~` before the
  name to truncate the module name for public names (eventually, we will have
  *only* public names in this part of documentation, see
  sympy#22105, but this is a separate issue to
  clean this up).

- When building, be aware that files for the autosummary are generated
  automatically, but are only cleaned when you do a `make clean`. Thus if you
  add something to an autosummary and then delete it, you will get warnings in
  the build from the generated file that no longer is referenced anywhere.
@sympy-bot
Copy link

sympy-bot commented Dec 3, 2021

Hi, I am the SymPy bot (v162). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • other
    • Start using the autosummary extension in the docs, which puts documentation for each function and class on a separate page. (#22589 by @asmeurer)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.10.

Click here to see the pull request description that was parsed.
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

This is a redux of the work that was started in https://github.com/sympy/sympy/pull/18594. 

#### Brief description of what is fixed or changed

Our [documentation poll](
https://forms.gle/EjkFEdnLXaYxjvfn6) is still ongoing, but one thing that is already becoming clear from some of the responses is that our documentation would benefit from splitting the documentation of each function into separate pages. This is achieved with the autosummary Sphinx extension. 

Not only does this make each page of the documentation more manageable, but it also makes it easier for Google search results to return the exact function in question, rather than just a large page that contains the function somewhere on it. 

For now, I have only modified the matrices.rst file. Please review what this looks like, both in the RST and the generated HTML. If we like how this looks, we can add it to other pages.

#### Other comments


- You have to manually add `:toctree: ref` to each autosummary to tell it where
  to put the autogenerated files. You should always use the name "ref" for
  this for consistency, but you can use something like `:toctree: ../ref` to put
  it on level up. The word "ref" is part in the URL for each generated page.

- The classes I have documented here really need to be organized in a better
  way. However, I have not done that in this PR. An advantage of autosummary
  is that we can move where a class is documented in the organization without
  breaking the URL to it, since it lives on a separate page.

- This will break any existing links to specific anchors on this page.

- The class template hardcodes :members:. However, it would be easy to use
  other combinations by adding additional templates, although each autosummary
  list can only use one template at a time.

- The docs for classes still list each method on the same page. This is
  because autosummary is not really capable of doing recursive autosummary on
  class members in a way that only includes the members we want to include.
  See the commit message of a0e955bfc56267877442ab318e9ec92f29a806d6.

- Autosummary is very particular about how you spell the name of the function
  so that it can find it. I recommend avoiding `.. module::` or `..
  currentmodule::`and just spelling everything explicitly, using `~` before the
  name to truncate the module name for public names (eventually, we will have
  *only* public names in this part of documentation, see
  #22105, but this is a separate issue to
  clean this up).

- When building, be aware that files for the autosummary are generated
  automatically, but are only cleaned when you do a `make clean`. Thus if you
  add something to an autosummary and then delete it, you will get warnings in
  the build from the generated file that no longer is referenced anywhere.

#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
- other
  - Start using the autosummary extension in the docs, which puts documentation for each function and class on a separate page. 
<!-- END RELEASE NOTES -->

#### Screenshots

Here is what the matrices page looks like
<img width="838" alt="Screen Shot 2021-12-02 at 7 38 34 PM" src="https://user-images.githubusercontent.com/71486/144535550-40ee9895-6a1e-4b94-974b-c1085003d51a.png">

and here is what a page for one of the clases looks like

<img width="1072" alt="Screen Shot 2021-12-02 at 7 38 59 PM" src="https://user-images.githubusercontent.com/71486/144535579-8e1f3105-84bf-4d11-ad5e-38f5a64f6761.png">

@sympy-bot
Copy link

sympy-bot commented Dec 3, 2021

🟠

Hi, I am the SymPy bot (v162). I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.

This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at sympy/sympy-bot#75.

The following commits add new files:

  • a0e955b:
    • doc/src/_templates/autosummary/class.rst

If these files were added/deleted on purpose, you can ignore this message.

@asmeurer asmeurer added CZI: Documentation Issues and pull requests relating to Aaron Meurer's CZI grant documentation project Documentation labels Dec 3, 2021
@github-actions
Copy link

github-actions bot commented Dec 3, 2021

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

       before           after         ratio
     [907895ac]       [95f0228c]
+     6.74±0.01ms      10.3±0.05ms     1.52  matrices.TimeMatrixPower.time_Case1
-      4.30±0.02s          319±4ms     0.07  polygon.PolygonArbitraryPoint.time_bench01
+     3.34±0.04ms      5.60±0.07ms     1.67  solve.TimeMatrixOperations.time_det(4, 2)
+     3.32±0.02ms      5.59±0.08ms     1.69  solve.TimeMatrixOperations.time_det_bareiss(4, 2)

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

@asmeurer
Copy link
Member Author

asmeurer commented Dec 3, 2021

I've noticed a somewhat important limitation of this, as it's currently implemented. This puts the fully qualified name as the title of every autosummary page. This also goes in the URL, so you'd get something like docs.sympy.org/latest/modules/matrices/ref/sympy.matrices.dense.ones.html instead of just docs.sympy.org/latest/modules/matrices/ref/ones.html (and it would also have sympy.matrices.dense.ones as the title of the page). This is bad because it would encourage people to import top-level names from submodules. We're already not great at distinguishing these anyway, but a bigger problem is that with this, it affects the URL too. So we ever wanted to move the function around or rename the submodule, it would break the URL.

I think I might have a workaround for this, by making separate "public" and "private" templates. I'll have to play with it tomorrow.

@@ -1,8 +1,6 @@
Matrices (linear algebra)
=========================

.. module:: sympy.matrices.matrices

Creating Matrices
-----------------
Copy link
Contributor

Choose a reason for hiding this comment

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

All of the content here above the autodoc lines at the bottom of the rst file should be moved into a separate rst file that could be linked to from here (perhaps a guide). The number one benefit of using autosummary is making all the different functions/methods more discoverable so we need it to be clear when you come to this page that it has a list of those functions which is not yet immediately obvious because I still need to scroll through a load of stuff to see that list.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. I wanted to minimize the diff so that we can focus on just the specific details about autosummary to make sure everything looks tenable before going all in on it. So far there are some concerns that need addressing, like the one I mentioned above.

@oscarbenjamin
Copy link
Contributor

The basic idea here looks good. In combination with the other changes to the top level organisation of the docs I think we're getting to something much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CZI: Documentation Issues and pull requests relating to Aaron Meurer's CZI grant documentation project Documentation Merge conflict
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants