Split up documentation into subpages and clean up some warnings by mattip · Pull Request #37419 · pytorch/pytorch

mattip · 2020-04-28T17:01:47Z

This is a major refactor of parts of the documentation to split it up using sphinx's autosummary feature which will build out autofuction and autoclass stub files and link to them. The end result is that the top module pages like torch.nn.rst and torch.rst are now more like table-of-contents to the actual single-class or single-function documentations pages.

Along the way, I modified many of the docstrings to eliminate sphinx warnings when building. I think the only thing I changed from a non-documentation perspective is to add names to __all__ when adding them to globals() in torch.__init__.py

I do not know the CI system: are the documentation build artifacts available after the build, so reviewers can preview before merging?

dr-ci · 2020-04-28T17:13:35Z

💊 Build failures summary and remediations

As of commit c277285 (more details on the Dr. CI page):

1/1 failures possibly* introduced in this PR
- 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed

Failed: pr/py3.6-clang7-rocmdeb-ubuntu16.04

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 27 times.

ShawnZhong · 2020-04-29T16:46:26Z

I really like the way you organized the functions to the generated folder and the changes you made to the docs.

FYI, I built the docs based on 2fd0354, and published it at https://shawnzhong.github.io/pytorch/.

It also turns out that the folder structure works pretty well with PyCharm/Inteiilj's External Documentation (See related discussion on PyTorch Forum) by setting the URL to https://shawnzhong.github.io/pytorch/generated/{element.qname}.html

rgommers · 2020-04-29T19:02:13Z

    to 'mode', resetting it when we exit the with-block.  A no-op if
    mode is None.
+
+    In verison 1.5 changed to this from set_training


minor: typo in version here. also set_training looks like it needs double backticks

ok, should be 1.6

and "version" not "verison"

rgommers

This looks quite good and from some testing it seems like this resolves both the searching and the "docs are heavy" issues well.

Visually I think the vertical margins on every function/class entry in autosummary tables could be made smaller, that way things get more compact and it would read even better. Non-blocking comment though, it's a matter of taste and either way this is a significant improvement.

rgommers · 2020-04-29T19:38:24Z

I do not know the CI system: are the documentation build artifacts available after the build, so reviewers can preview before merging?

There's a CircleCI job pytorch_python_doc_push that builds the docs, however it doesn't store any artifacts for PRs. Controlled by .circleci/scripts/python_doc_push_script.sh. I think that's the only doc build (?). Not sure why it doesn't store the artifacts, that would be useful.

mattip · 2020-04-29T19:59:54Z

TL DR: there is a broken test. I propose redoing it properly which will require a few days work.

The test/test_docs_coverage.py test is failing since it does this:

scan a top-level module's rst file for autofunction directives to determine which functions are documented
scan [a.__doc__ for a in dir(module)] for the string .. function in the docstring to determine which functions should be documented
compare the two, they should be equal.

This PR broke both sides of the equation:

autosummary replaces the autofunction directives and builds out stub files that in turn are linked back in, so now the documentation has a multi-level heirarchy
the leading .. function directive broke autosummary so I removed it.

I propose we use sphinx.ext.coverage to do this for us, but it means investing time to get the call right and parse the output. Are there other options? Should we add a make coverage CI step (which will also require work) to check coverage instead of a test? Should we disable the check entirely for now, and open a new issue to fix it?

ShawnZhong · 2020-04-30T05:17:34Z

I was checking the links for the functions and found that some overloaded functions are not linked correctly in torch.html

For those functions, the second column shows the function signature instead of the description, and the link target is set to torch.html#torch.mul instead of generated/point/torch.mul.html?highlight=mul

ShawnZhong · 2020-04-30T05:26:50Z

-.. autofunction:: trunc
-
+.. autosummary::
+    :toctree: generated/point


Maybe you want to use generated?

Not sure if it's just me, but it seems that this change doesn't fix the link issue mentioned above: torch.mul still links to the torch.html#torch.mul.

Strange. I don't see this on two different builds (both with cuda installed, one on a machine with a GPU, one without). There is nothing obvious in torch/_torch_docs.py in the docstrings for these functions as overloaded from add_docstr(torch.mul,... and friends. What do you mean by "some overloaded functions" - is there something special about mul and pow that my builds are not picking up?

How are you building and uploading the docs to https://shawnzhong.github.io/pytorch ?

I guess I could try to replicate the actual python_doc_push_script.sh in a docker, does that CI step run with a GPU?

I use a GitHub action to build and push the HTML files to gh_pages, which is served by GitHub pages.

It might be a different environment that causes this issue. I tried to use the same environment as CircileCI, but it seems that I don't have the permission to pull the docker image for building docs. BTW, I just opened a PR to upload the build artifact for python docs at #37658.

Nvm, it works on the CircleCI build. https://5319560-65600975-gh.circle-artifacts.com/0/~/workspace/build_artifacts/master/index.html

ezyang · 2020-04-30T14:10:16Z

I propose we use sphinx.ext.coverage to do this for us, but it means investing time to get the call right and parse the output. Are there other options? Should we add a make coverage CI step (which will also require work) to check coverage instead of a test? Should we disable the check entirely for now, and open a new issue to fix it?

Reading over the sphinx coverage docs, it does seem like this is the right thing to do. You can see the original PR #16039 we mostly added this test script in anger. The important thing is the test needs to fail when you don't document and don't explicitly blacklist. The only plausible alternative is to bulk up the script so that it can handle hierarchy on the documentation; probably not too hard either, but if Sphinx has a built in to do this, we probably should use it.

However, there's a huge benefit to getting this PR in directly, so I think that it is OK not to block this PR and disable the check for now, and then fix it later.

cc @zasdfgbnm who originally added the test

zasdfgbnm · 2020-04-30T16:43:24Z

I propose we use sphinx.ext.coverage to do this for us, but it means investing time to get the call right and parse the output. Are there other options? Should we add a make coverage CI step (which will also require work) to check coverage instead of a test? Should we disable the check entirely for now, and open a new issue to fix it?

Reading over the sphinx coverage docs, it does seem like this is the right thing to do. You can see the original PR #16039 we mostly added this test script in anger. The important thing is the test needs to fail when you don't document and don't explicitly blacklist. The only plausible alternative is to bulk up the script so that it can handle hierarchy on the documentation; probably not too hard either, but if Sphinx has a built in to do this, we probably should use it.

However, there's a huge benefit to getting this PR in directly, so I think that it is OK not to block this PR and disable the check for now, and then fix it later.

cc @zasdfgbnm who originally added the test

I agree that it is a good idea to just disable this test to unblock this. After you disable it, can you open an issue for that?

…mary

mattip · 2020-04-30T17:35:00Z

test disabled. xref issue Rewrite documentation coverage tests #37590
fixed (via review comment)

some overloaded functions are not linked correctly in torch.html
rebased to pick up new vander function

ezyang · 2020-04-30T22:11:50Z

        Labelling Unsegmented Sequence Data with Recurrent Neural Networks:
        https://www.cs.toronto.edu/~graves/icml_2006.pdf

-    .. Note::


Huh, .. Note:: is not actually a thing?

.. note:: is a directive for general RST and rendered as a <div> with formatting to look like a box, Note: is a docstring heading. I think the intent here is a docstring section

facebook-github-bot

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: This PR allows the build artifacts for python docs to be stored on CircieCI, which helps the reviewer to preview doc changes before merging. The artifacts can be found in the [`ARTIFACTS` tab]( https://app.circleci.com/pipelines/github/pytorch/pytorch/162986/workflows/a969f256-3243-414f-8a02-1234b9dac149/jobs/5320907/artifacts) of the test **pytorch_cpp_doc_push**, and the website is served at https://5320907-65600975-gh.circle-artifacts.com/0/docs/index.html This PR is inspired by rgommers's comment under pytorch#37419 (comment) > There's a CircleCI job pytorch_python_doc_push that builds the docs, however it doesn't store any artifacts for PRs. Controlled by .circleci/scripts/python_doc_push_script.sh. I think that's the only doc build (?). Not sure why it doesn't store the artifacts, that would be useful. Pull Request resolved: pytorch#37658 Differential Revision: D21380094 Pulled By: ezyang fbshipit-source-id: 1dd44bf836ebc74454f4444ae9321807dccdb313

…rch#37419) Summary: xref pytorchgh-32838, pytorchgh-34032 This is a major refactor of parts of the documentation to split it up using sphinx's `autosummary` feature which will build out `autofuction` and `autoclass` stub files and link to them. The end result is that the top module pages like torch.nn.rst and torch.rst are now more like table-of-contents to the actual single-class or single-function documentations pages. Along the way, I modified many of the docstrings to eliminate sphinx warnings when building. I think the only thing I changed from a non-documentation perspective is to add names to `__all__` when adding them to `globals()` in `torch.__init__.py` I do not know the CI system: are the documentation build artifacts available after the build, so reviewers can preview before merging? Pull Request resolved: pytorch#37419 Differential Revision: D21337640 Pulled By: ezyang fbshipit-source-id: d4ad198780c3ae7a96a9f22651e00ff2d31a0c0f

…ubpages and clean up some warnings" (pytorch#37778) Summary: Original PR: pytorch#37419 cc mattip suo Pull Request resolved: pytorch#37778 Differential Revision: D21385774 Pulled By: ezyang fbshipit-source-id: 5de532faab8bae132736b6b5189e0ee2ac9935be

Summary: This PR allows the build artifacts for python docs to be stored on CircieCI, which helps the reviewer to preview doc changes before merging. The artifacts can be found in the [`ARTIFACTS` tab]( https://app.circleci.com/pipelines/github/pytorch/pytorch/162986/workflows/a969f256-3243-414f-8a02-1234b9dac149/jobs/5320907/artifacts) of the test **pytorch_cpp_doc_push**, and the website is served at https://5320907-65600975-gh.circle-artifacts.com/0/docs/index.html This PR is inspired by rgommers's comment under pytorch#37419 (comment) > There's a CircleCI job pytorch_python_doc_push that builds the docs, however it doesn't store any artifacts for PRs. Controlled by .circleci/scripts/python_doc_push_script.sh. I think that's the only doc build (?). Not sure why it doesn't store the artifacts, that would be useful. Pull Request resolved: pytorch#37658 Differential Revision: D21380094 Pulled By: ezyang fbshipit-source-id: 1dd44bf836ebc74454f4444ae9321807dccdb313

…rch#37419) Summary: xref pytorchgh-32838, pytorchgh-34032 This is a major refactor of parts of the documentation to split it up using sphinx's `autosummary` feature which will build out `autofuction` and `autoclass` stub files and link to them. The end result is that the top module pages like torch.nn.rst and torch.rst are now more like table-of-contents to the actual single-class or single-function documentations pages. Along the way, I modified many of the docstrings to eliminate sphinx warnings when building. I think the only thing I changed from a non-documentation perspective is to add names to `__all__` when adding them to `globals()` in `torch.__init__.py` I do not know the CI system: are the documentation build artifacts available after the build, so reviewers can preview before merging? Pull Request resolved: pytorch#37419 Differential Revision: D21337640 Pulled By: ezyang fbshipit-source-id: d4ad198780c3ae7a96a9f22651e00ff2d31a0c0f

…ubpages and clean up some warnings" (pytorch#37778) Summary: Original PR: pytorch#37419 cc mattip suo Pull Request resolved: pytorch#37778 Differential Revision: D21385774 Pulled By: ezyang fbshipit-source-id: 5de532faab8bae132736b6b5189e0ee2ac9935be

mattip · 2020-05-06T11:08:46Z

I am not sure whether to close gh-32838, gh-34032. There is (and probbly will always be) more to do here:

quantization.html and maybe other pages still could be broken into smaller units.
remaining warnings should be fixed, and we could turn on sphinx warning-as-error once warnings reach 0 in order to prevent new warnings from appearing
css and SEO improvements probably better left to web designers:
- left navbar: subtopic indentation, using standard triangles to collapse topics,
- right navbar that appears in subtopics should be used in sub-subtopics as well
- keywords to help search engines
- robot.txt to prevent search engines finding older versions of the documentation
I did not look at the site on mobile

ezyang · 2020-05-06T20:56:26Z

I think it's ok to file new issues for these, since the main issue has been addressed.

mattip · 2020-05-07T12:33:34Z

xref
gh-38010 to split more files
gh-38011 to remove warnings
gh-38012 to improve formatting.
I see that searching for "pytorch autograd" already points to the stable documentation, so SEO seems OK.

I think we can close the main issues gh-32838, gh-34032 now.

mrshenli · 2020-06-15T18:23:43Z

Hey @mattip, we noticed that rpc/index.rst is removed from this PR. That page is the landing page for the RPC package. Is there any plan for adding it back before v1.6 branch cut?

cc @rohan-varma @jlin27

mattip · 2020-06-15T18:44:47Z

That page is the landing page for the RPC package

Since much of the content of docs/source/rpc/index.rst page was repeated on docs/source/rpc.rst I took the liberty of consolidating them into the latter. If there are external references to the former and not the latter, maybe we should switch it around? It seemed strange to me that there was an index.rst page but no other pages. The content is rendered at https://pytorch.org/docs/master/rpc.html.

mrshenli · 2020-06-15T19:14:22Z

Hey @mattip

Thanks for the info. And the change looks OK to me. Just one concern, it seems like the RPC package disappeared from the main indexing page (and the left navigation bar) on master: https://pytorch.org/docs/master/

Do we plan to fix that?

In the stable release (https://pytorch.org/docs/stable/), there is a Distributed RPC Framework link points to the rpc.rst page.

Summary: Fixes gh-40046 PR gh-37419 refactored the content of `docs/source/rpc/index.rst` into `docs/source/rpc.rst` but did not link to the latter from `doc/source/index.rst` so the top-level RPC documentation is missing from https://pytorch.org/docs/master/. Pull Request resolved: #40077 Differential Revision: D22068128 Pulled By: mrshenli fbshipit-source-id: 394433f98f86509e0c9cb6d910a86fb8a2932683

Summary: Fixes pytorchgh-40046 PR pytorchgh-37419 refactored the content of `docs/source/rpc/index.rst` into `docs/source/rpc.rst` but did not link to the latter from `doc/source/index.rst` so the top-level RPC documentation is missing from https://pytorch.org/docs/master/. Pull Request resolved: pytorch#40077 Differential Revision: D22068128 Pulled By: mrshenli fbshipit-source-id: 394433f98f86509e0c9cb6d910a86fb8a2932683

Summary: This PR allows the build artifacts for python docs to be stored on CircieCI, which helps the reviewer to preview doc changes before merging. The artifacts can be found in the [`ARTIFACTS` tab]( https://app.circleci.com/pipelines/github/pytorch/pytorch/162986/workflows/a969f256-3243-414f-8a02-1234b9dac149/jobs/5320907/artifacts) of the test **pytorch_cpp_doc_push**, and the website is served at https://5320907-65600975-gh.circle-artifacts.com/0/docs/index.html This PR is inspired by rgommers's comment under pytorch#37419 (comment) > There's a CircleCI job pytorch_python_doc_push that builds the docs, however it doesn't store any artifacts for PRs. Controlled by .circleci/scripts/python_doc_push_script.sh. I think that's the only doc build (?). Not sure why it doesn't store the artifacts, that would be useful. Pull Request resolved: pytorch#37658 Differential Revision: D21380094 Pulled By: ezyang fbshipit-source-id: 1dd44bf836ebc74454f4444ae9321807dccdb313

…rch#37419) Summary: xref pytorchgh-32838, pytorchgh-34032 This is a major refactor of parts of the documentation to split it up using sphinx's `autosummary` feature which will build out `autofuction` and `autoclass` stub files and link to them. The end result is that the top module pages like torch.nn.rst and torch.rst are now more like table-of-contents to the actual single-class or single-function documentations pages. Along the way, I modified many of the docstrings to eliminate sphinx warnings when building. I think the only thing I changed from a non-documentation perspective is to add names to `__all__` when adding them to `globals()` in `torch.__init__.py` I do not know the CI system: are the documentation build artifacts available after the build, so reviewers can preview before merging? Pull Request resolved: pytorch#37419 Differential Revision: D21337640 Pulled By: ezyang fbshipit-source-id: d4ad198780c3ae7a96a9f22651e00ff2d31a0c0f

…ubpages and clean up some warnings" (pytorch#37778) Summary: Original PR: pytorch#37419 cc mattip suo Pull Request resolved: pytorch#37778 Differential Revision: D21385774 Pulled By: ezyang fbshipit-source-id: 5de532faab8bae132736b6b5189e0ee2ac9935be

Summary: Fixes pytorchgh-40046 PR pytorchgh-37419 refactored the content of `docs/source/rpc/index.rst` into `docs/source/rpc.rst` but did not link to the latter from `doc/source/index.rst` so the top-level RPC documentation is missing from https://pytorch.org/docs/master/. Pull Request resolved: pytorch#40077 Differential Revision: D22068128 Pulled By: mrshenli fbshipit-source-id: 394433f98f86509e0c9cb6d910a86fb8a2932683

mattip requested review from albanD and apaszke as code owners April 28, 2020 17:01

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Apr 28, 2020

albanD reviewed Apr 28, 2020

View reviewed changes

Comment thread torch/autograd/functional.py

pytorchbot added the open source label Apr 28, 2020

mattip changed the title ~~Issue 32838~~ Split up documentation into subpages and clean up some warnings Apr 29, 2020

rgommers reviewed Apr 29, 2020

View reviewed changes

rgommers requested a review from ezyang April 29, 2020 19:30

ShawnZhong reviewed Apr 30, 2020

View reviewed changes

mattip mentioned this pull request Apr 30, 2020

Rewrite documentation coverage tests #37590

Closed

mattip added 6 commits April 30, 2020 20:29

refactor nn.rst into sections and clean up some warnings

d60cd9d

DOC: replace ..include in docstrings

fb2eb48

reflow some pages, fix many warnings. Refactor jit.rst to use autosum…

78da98b

…mary

refactor quantization, jit, torch to reduce warnings and split up torch

089e176

restore mistakenly deleted content and fix flake8 errors

43ea41f

review fixes, refactor rpc and java torchvision documentation inclusion

5c5167c

mattip force-pushed the issue-32838 branch from 1e65c60 to 5c5167c Compare April 30, 2020 17:32

ezyang reviewed Apr 30, 2020

View reviewed changes

ezyang approved these changes Apr 30, 2020

View reviewed changes

facebook-github-bot reviewed Apr 30, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into issue-32838

68c05c2

This was referenced May 7, 2020

Split more pages into sub-topics #38010

Open

Formatting of topic and sub-topic pages #38012

Open

ShawnZhong mentioned this pull request Jun 11, 2020

Feedback to split certain doc pages into sub-topics #39836

Open

mrshenli mentioned this pull request Jun 15, 2020

split up community documentation, add build/test document #38629

Closed

This was referenced Jun 15, 2020

Restore top-level link to RPC #40046

Closed

DOC: fix rpc reference in top-level index #40077

Closed

mruberry added the Merged label Oct 28, 2020

Conversation

mattip commented Apr 28, 2020

Uh oh!

Uh oh!

dr-ci Bot commented Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 Build failures summary and remediations

ci.pytorch.org: 1 failed

Uh oh!

ShawnZhong commented Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

rgommers commented Apr 29, 2020

Uh oh!

mattip commented Apr 29, 2020

Uh oh!

ShawnZhong commented Apr 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezyang commented Apr 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zasdfgbnm commented Apr 30, 2020

Uh oh!

mattip commented Apr 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

mattip commented May 6, 2020

Uh oh!

ezyang commented May 6, 2020

Uh oh!

mattip commented May 7, 2020

Uh oh!

mrshenli commented Jun 15, 2020

Uh oh!

mattip commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrshenli commented Jun 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

dr-ci Bot commented Apr 28, 2020 •

edited

Loading

ShawnZhong commented Apr 29, 2020 •

edited

Loading

ezyang commented Apr 30, 2020 •

edited

Loading

mattip commented Jun 15, 2020 •

edited

Loading