Polish CPP docs, Minor Python Docs Fixes#11722
Closed
svenevs wants to merge 11 commits intopytorch:masterfrom
svenevs:polish-cpp-docs
Closed
Polish CPP docs, Minor Python Docs Fixes#11722svenevs wants to merge 11 commits intopytorch:masterfrom svenevs:polish-cpp-docs
svenevs wants to merge 11 commits intopytorch:masterfrom
svenevs:polish-cpp-docs
Conversation
- py docs: add `-f` to avoid failure when `build/` does not exist - cpp docs: must be before catch-all and remove generated `api/`
Contributor
|
Thanks! 😊 I'll keep an eye on the PR |
- Doxygen may provide multiple warnings about items in the build directory - `grep -v` can give an exit code of 1 if warning match not found which will halt script from `set -e` at top.
- remove unsupported use of `html_context` - let `html_theme` set `html_theme_path` (Sphinx 1.6+)
- remove unsupported use of `html_context` - let `html_theme` set its `html_theme_path` (Sphinx 1.6+)
doxygen is choking on /// ... /*slope=0.5*/ for inline comment
goldsborough
approved these changes
Sep 18, 2018
Contributor
goldsborough
left a comment
There was a problem hiding this comment.
Infinite gratitude for working on this! 😊 Waiting for @soumith to stamp the Python doc changes
Contributor
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 18, 2018
Differential Revision: D9919120 Pulled By: goldsborough fbshipit-source-id: bf14cbe4ab79524495957cb749828046af864aab
Contributor
|
Considering this has been merged, is there any reason to keep it open? |
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CC: @goldsborough
Generic CPP Docs Stuff
Clean targets
-fso that a vanillamake cleandoesn't formally fail ;)Simplify
Doxyfilefor maintainers 💥CPP: separate
sourceandbuilddirectories.cd docs/cpp && make htmlfollowed by copyingdocs/cpp/build/html/**.See
HTML_OUTPUTinDoxyfilefor how to easily compare doxygen html with Sphinxfix
FunctionImpldocs (nesting document-comments///and/*scale=0.5*/apparently confuses doxygen.remove forward declaration to guarantee correct location extracted by exhale
CPP and Python
conf.pyCleanup / HTML Theme "Fixes"Separating into two categories since I cannot figure out how the python docs build. Somebody needs to test them and report back success or failure.
Removal of
html_context['css_files']Using
html_context['css_files']is apparently formally undefined behavior. See sphinx/#2442. Unfortunately, this took a long time to have an official fix in Sphinx. Sphinx 1.8 and later you can usehtml_css_files, but since the python docs are pinned at1.7.9(as well as CPP docs will break with Sphinx 1.8 if more files are documented), I'm adding the "old"setup(app)method. The sphinx 1.8 // cpp docs issues are something in breathe that will take a lot of effort to fix 😢Removal of
html_static_pathSetting
html_theme_pathis no longer necessary, Sphinx 1.6+ allow html theme's to set this in theirsetup(app)function (and the sphinx rtd theme does it correctly). In some (rare) cases it may actually create problems / conflicts.I guess officially you could introduce convoluted logic to check if Sphinx <1.6 but I view that as pointless.
CPP docs html setup fixes / cleanup
html_context['css_files']removalhtml_static_pathPython: html setup fixes / cleanup
html_context['css_files']removalhtml_static_path