-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add make docs
and doc building instructions
#9534
Conversation
f1474b3
to
d5a83e3
Compare
docs/conf.py
Outdated
@@ -352,7 +352,7 @@ def force_gc(gallery_conf, fname): | |||
"gallery_dirs": gallery_dirs, | |||
"subsection_order": subsection_order, | |||
"filename_pattern": os.environ.get("TVM_TUTORIAL_EXEC_PATTERN", ".py"), | |||
"find_mayavi_figures": False, | |||
"image_scrapers": ("mayavi",), |
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 should be equivalent on recent versions of Sphinx: https://github.com/sphinx-gallery/sphinx-gallery/blob/master/sphinx_gallery/gen_gallery.py#L150-L156
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.
just curious, what was the problem being solved by adding this here? right now we're mostly worried about the Sphinx version pinned in ci-gpu, so while I think it's good for us to support a more recent sphinx, we need to dig ourselves out of relying on ci-gpu for all docs purposes (e.g. and use it only when doing a full docs build including gpu-dependent galleries) before we should think about making the docs flow work on a modern ubuntu.
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.
just laziness, initially I was using a more recent version of Sphinx that needed this fix to run but with using the versions from ci-gpu this isn't necessary
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.
thanks @driazati, a couple questions. moving the README.txt to markdown is a great idea.
docs/requirements.txt
Outdated
@@ -0,0 +1,11 @@ | |||
autodocsumm==0.2.7 |
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.
rather than do this, should we add a tool to run pip freeze on ci-gpu and then use this as a constraints file to locally install these deps? my concern is that adding a requirements.txt with pins may drift.
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.
Removed the requirements.txt and added 2 building flows, one that uses a short script and relies on ci-gpu and one that just uses it to generate the list of dependencies. Given that creating a list is pretty simple and anyone developing the docs probably has access to Docker I think it's fine to not commit the dependencies list for now and let people generate it directly.
For now this works OK but the makefile + bash setup doesn't seem tenable for the larger goals of allowing developers to reproduce CI easily, we might want to consider a Python runner similar to Rust's added a Python runner insteadx.py
docs/conf.py
Outdated
@@ -352,7 +352,7 @@ def force_gc(gallery_conf, fname): | |||
"gallery_dirs": gallery_dirs, | |||
"subsection_order": subsection_order, | |||
"filename_pattern": os.environ.get("TVM_TUTORIAL_EXEC_PATTERN", ".py"), | |||
"find_mayavi_figures": False, | |||
"image_scrapers": ("mayavi",), |
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.
just curious, what was the problem being solved by adding this here? right now we're mostly worried about the Sphinx version pinned in ci-gpu, so while I think it's good for us to support a more recent sphinx, we need to dig ourselves out of relying on ci-gpu for all docs purposes (e.g. and use it only when doing a full docs build including gpu-dependent galleries) before we should think about making the docs flow work on a modern ubuntu.
d5a83e3
to
f3734a2
Compare
57a995b
to
fe8b1b4
Compare
8f46b71
to
6a7c994
Compare
make docs
and doc building instructions
6a7c994
to
fea5b4a
Compare
fea5b4a
to
4a1fc4f
Compare
Thanks @driazati . Considering the fact that not everybody have a GPU machine in their env. It would be good to "promote" the instruction for building docs without running tutorials(which can actually be most of the cases). This will also answer the common question "how do i build docs quickly on my local env in a few mins" https://github.com/apache/tvm/blob/main/tests/scripts/task_sphinx_precheck.sh#L39 does that/ |
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.
thanks @driazati, did a round of review here
5cb5241
to
d9b81f6
Compare
This pins the dependencies for the docs and adds `pytest` as a dependency which was missing when I built. Tested out the requirements.txt with a fresh `ubuntu:focal` Docker image to verify that the required depedencies work.
3816e0e
to
4b7aaca
Compare
* Update doc building instructions and pin dependencies This pins the dependencies for the docs and adds `pytest` as a dependency which was missing when I built. Tested out the requirements.txt with a fresh `ubuntu:focal` Docker image to verify that the required depedencies work. * Address comments, add Makefile for docs and add to instructions * Use Python for running scripts * Fix lint, add lint command * Add option for cpu to only run the precheck, address comments * Fix 'make doc' usage, add some -x's * Fix bad condition on --cpu, add defaults for envs * Fix another 'make doc' * Fix for running on MacOS Co-authored-by: driazati <driazati@users.noreply.github.com>
* Update doc building instructions and pin dependencies This pins the dependencies for the docs and adds `pytest` as a dependency which was missing when I built. Tested out the requirements.txt with a fresh `ubuntu:focal` Docker image to verify that the required depedencies work. * Address comments, add Makefile for docs and add to instructions * Use Python for running scripts * Fix lint, add lint command * Add option for cpu to only run the precheck, address comments * Fix 'make doc' usage, add some -x's * Fix bad condition on --cpu, add defaults for envs * Fix another 'make doc' * Fix for running on MacOS Co-authored-by: driazati <driazati@users.noreply.github.com>
* Update doc building instructions and pin dependencies This pins the dependencies for the docs and adds `pytest` as a dependency which was missing when I built. Tested out the requirements.txt with a fresh `ubuntu:focal` Docker image to verify that the required depedencies work. * Address comments, add Makefile for docs and add to instructions * Use Python for running scripts * Fix lint, add lint command * Add option for cpu to only run the precheck, address comments * Fix 'make doc' usage, add some -x's * Fix bad condition on --cpu, add defaults for envs * Fix another 'make doc' * Fix for running on MacOS Co-authored-by: driazati <driazati@users.noreply.github.com>
* Update doc building instructions and pin dependencies This pins the dependencies for the docs and adds `pytest` as a dependency which was missing when I built. Tested out the requirements.txt with a fresh `ubuntu:focal` Docker image to verify that the required depedencies work. * Address comments, add Makefile for docs and add to instructions * Use Python for running scripts * Fix lint, add lint command * Add option for cpu to only run the precheck, address comments * Fix 'make doc' usage, add some -x's * Fix bad condition on --cpu, add defaults for envs * Fix another 'make doc' * Fix for running on MacOS Co-authored-by: driazati <driazati@users.noreply.github.com>
* Update doc building instructions and pin dependencies This pins the dependencies for the docs and adds `pytest` as a dependency which was missing when I built. Tested out the requirements.txt with a fresh `ubuntu:focal` Docker image to verify that the required depedencies work. * Address comments, add Makefile for docs and add to instructions * Use Python for running scripts * Fix lint, add lint command * Add option for cpu to only run the precheck, address comments * Fix 'make doc' usage, add some -x's * Fix bad condition on --cpu, add defaults for envs * Fix another 'make doc' * Fix for running on MacOS Co-authored-by: driazati <driazati@users.noreply.github.com>
This moves the doc instructions to a
.md
so GitHub knows how to render it nicely in the browser. It also adds a Python runnertests/scripts/ci.py
that will invoke the same scripts as in CI to build the docs, along with some options to make local development nicer (i.e. most people probably don't want to wait around for the C++/typescript docs to build if they are editing the Python docs). Simple usage can be done from the makefile with the defaults, or the script can be invoked directly for more control