Skip to content

doc: Set theme in conf.py #1239

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

Merged
merged 6 commits into from
Apr 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,43 @@ You can also run them in parallel:
tox -- -n auto tests/integ


Building Sphinx docs
~~~~~~~~~~~~~~~~~~~~

Setup a Python environment with ``sphinx`` and ``sagemaker``:

::

conda create -n sagemaker python=3.7
conda activate sagemaker
conda install sphinx==2.2.2
pip install sagemaker --user

Install the Read The Docs theme:

::

pip install sphinx_rtd_theme --user
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would it be worth adding a "doc" entry to https://github.com/aws/sagemaker-python-sdk/blob/master/setup.py#L47-L55?

Mmmm no. IDK think it's a good idea to pop installing Sphinx on people. It has a lot of issues with versions and dependency incompatibilities. You could hose someone's setup by pushing a newer version of Sphinx and it's not critical to the core functionality of the sdk.

Copy link
Contributor

Choose a reason for hiding this comment

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

neither sphinx nor sphinx_rtd_theme is in setup.py right now for what it's worth. @nadiaya do you have thoughts on this?

Copy link
Contributor

@nadiaya nadiaya Jan 16, 2020

Choose a reason for hiding this comment

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

I agree with Aaron, sphinx is not essential to any part of py-sdk functionality. We shouldn't add it to dependencies.



Clone/fork the repo, ``cd`` into the ``sagemaker-python-sdk/doc`` directory and run:

::

make html

You can edit the templates for any of the pages in the docs by editing the .rst files in the ``doc`` directory and then running ``make html`` again.

Preview the site with a Python web server:

::

cd _build/html
python -m http.server 8000

View the website by visiting http://localhost:8000


MXNet SageMaker Estimators
--------------------------

Expand Down
7 changes: 2 additions & 5 deletions doc/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"""Placeholder docstring"""
from __future__ import absolute_import

import os
import pkg_resources
import sys
from datetime import datetime
Expand Down Expand Up @@ -83,10 +82,8 @@ def __getattr__(cls, name):
autodoc_default_flags = ["show-inheritance", "members", "undoc-members"]
autodoc_member_order = "bysource"

if "READTHEDOCS" in os.environ:
html_theme = "default"
else:
html_theme = "haiku"
html_theme = "sphinx_rtd_theme"

html_static_path = []
htmlhelp_basename = "%sdoc" % project

Expand Down