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

Documentation bugs #101

Closed
16 of 20 tasks
adamjstewart opened this issue Sep 3, 2021 · 8 comments
Closed
16 of 20 tasks

Documentation bugs #101

adamjstewart opened this issue Sep 3, 2021 · 8 comments
Labels
documentation Improvements or additions to documentation

Comments

@adamjstewart
Copy link
Collaborator

adamjstewart commented Sep 3, 2021

This is a running list of bugs I've noticed in the documentation that need to be fixed.

@adamjstewart adamjstewart added the documentation Improvements or additions to documentation label Sep 3, 2021
@calebrob6
Copy link
Member

There is some incompatibility going on between the pytorch_sphinx_theme and nbsphinx. With nbsphinx enabled in conf.py:

  • The sidebar navigation doesn't show expand/collapse buttons
  • The sidebar navigation doesn't scroll with the page
  • Sections don't have permalinks
  • There are require.js errors on the page

@calebrob6
Copy link
Member

^ the above issues I mentioned were resolved by adding nbsphinx_requirejs_path = "" in docs/conf.py

Some references:

@adamjstewart
Copy link
Collaborator Author

We could consider switching to the trojanzoo_sphinx_theme which is forked off of pytorch_sphinx_theme but better maintained.

@adamjstewart
Copy link
Collaborator Author

If we do switch to trojanzoo_sphinx_theme, we'll be able to move docs/requirements.txt into setup.cfg

@adamjstewart
Copy link
Collaborator Author

__getitem__ is showing up before __init__ because things are sorted alphabetically, we don't want this

I believe this is caused by sphinx-doc/sphinx#9395. We're using the correct settings in our conf.py:

autodoc_member_order = "bysource"                                                        

However, Sphinx doesn't know where to locate the source code because we override __module__ in order for other things to work.

@robmarkcole
Copy link
Contributor

There is an error in trainers.ipynb. The csv is created with upper case column names (e.g. train_RMSE). Cell fixed below:

train_steps = []
train_rmse = []

val_steps = []
val_rmse = []
with open(os.path.join(experiment_dir, "tutorial_logs", "version_0", "metrics.csv"), "r") as f:
    csv_reader = csv.DictReader(f, delimiter=',')
    for i, row in enumerate(csv_reader):
        try:
            # train_rmse.append(float(row["train_rmse"]))
            train_rmse.append(float(row["train_RMSE"]))
            train_steps.append(i)
        except ValueError: # Ignore rows where train RMSE is empty 
            pass

        try:
            # val_rmse.append(float(row["val_rmse"]))
            val_rmse.append(float(row["val_RMSE"]))
            val_steps.append(i)
        except ValueError: # Ignore rows where val RMSE is empty
            pass

@calebrob6
Copy link
Member

Thanks for the catch / report @robmarkcole! This is one of the few things that isn't tested... Fixed in #434

@adamjstewart
Copy link
Collaborator Author

I think we can close this issue. Most remaining issues are documented in the pytorch-sphinx-theme repo. I want to change this in #2006

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants