-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fixed documentation inside the template #39
Conversation
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 looks good to me, but I'll let @agriyakhetarpal (pybamm's goto documentation person, haha) do the final review
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.
Looks good, just see a few comments
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 @santacodes, some comments below
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.
Looks good
@agriyakhetarpal do you have any ideas on what we could replace the |
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, @santacodes! I think it is better to remove the CSS file altogether and keep html_css_files = ["custom.css"]
commented out in conf.py
since the PyData Sphinx Theme already provides its own styles. The CSS file contains a lot of PyBaMM-specific configuration that users can copy manually if they wish to adjust and tinker with things. This is the only change I would request – the rest of the code looks good to me, and I'm glad to approve this PR after that.
Co-authored-by: Agriya Khetarpal <[email protected]>
The doctests would fail without something inside the In reference to this |
Oh, I see. In that case, we can add an empty |
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 for the fixes, @santacodes!
As brought up in this discussion the documentation should build now without any errors.