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

Add Sphinx documentation and publish to RTD. #177

Merged
merged 7 commits into from
Jan 21, 2025
Merged

Conversation

erinhmclark
Copy link
Collaborator

@erinhmclark erinhmclark commented Jan 16, 2025

Creating a fresh PR adding Sphinx documentation with RTD integration.

Summary

  • Adds Sphinx and autoapi
  • Add docs requirements group to poetry environment. We could further separate some such as autobuild into a docs-dev (or just move to dev?)
  • Currently contains a generated documentation file of configs. I haven't put this script into this PR as it's more a proof of concept and we're planning to restructure, but in theory this or the equivalent could be generated within the docs.

Run locally

poetry install

sphinx-build -b html docs/source docs/_build/html

Then either manually:
open docs/_build/html/index.html

Or run autobuild to automatically detect changes when modifying the docs:

poetry run sphinx-autobuild docs/source docs/_build/html

@erinhmclark
Copy link
Collaborator Author

Once we get the .readthedocs.yaml into main we can watch future builds on RTD.
The current README.md is loaded in the root of the docs, but still needs some internal paths fixed

@erinhmclark erinhmclark changed the title Feat/documentation Add Sphinx documentation and publish to RTD. Jan 16, 2025
@pjrobertson
Copy link
Collaborator

pjrobertson commented Jan 16, 2025

This looks great! Some minor comments (pretty minor - I think this can already be merged!)

  1. I got these two folders after building, do they need ignoring/commiting? /docs/build/ and /docs/source/autoapi/
  2. For me, the built files were put in docs/build/html/index.html not build/html/index.html as you state in the PR - is this an issue with my build settings or a typo?
  3. Configs seems redundant in the side panel, it's the same page as Configuration, and it's repeated twice:
Screen Region 2025-01-16 at 17 50 36
  1. I'd put User Guidelines at the top in the sidebar!
  2. Is there a way to make Api Guidelines not show the src part in the path?
Screen Region 2025-01-16 at 17 52 10
  1. How about a 'Building the Docs' section in the Readme? (p.s. the PR says use sphinx-build -b html docs/source docs/build/html but I had to add poetry run at the start of course

I was going to comment on some of the config docs, but I think since we will be changing this quite a lot, there's no need to change it right now.

@erinhmclark
Copy link
Collaborator Author

@pjrobertson most of those things were me lazily copying my instructions from different setups - apologies!

I will update the original description to fix those, and indeed docs/_make and docs/source/autobuild should be in .gitignore (I will do that in a min) and add a section to the README for it.

The Config docs were created by me running a hacky local script as mostly they're in the return statements rather than set as properties so I don't know if they can be pulled into the documentation via autoapi! If they do stay in a similar implementation we can tidy up that script and add it into the doc builds, but I think let's leave the automation out until then. However if there's something incorrect/ misleading I can manually edit that document too!

@pjrobertson
Copy link
Collaborator

Just one small comment on the 'Configs' section in the 'Configuration' file (can we also rename 'Configs' to something more intuitive)?

And then the removal of the src prefix from the autoapi info - I think this can be done by removing __init__.py from the src folder. It doesn't need to be there, and it looks like it is just a hangup from project restructuring anyway

Copy link
Collaborator

@pjrobertson pjrobertson left a comment

Choose a reason for hiding this comment

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

Great stuff!

@@ -0,0 +1,741 @@
Configs
Copy link
Collaborator

Choose a reason for hiding this comment

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

How were these .rst files created? Were they generated automatically, or by hand?

If automatically, perhaps we shouldn't include them in the repo, but should auto-generate them every time we build?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it's just this configs.rst file that includes the specific configurations that could be auto-generated. Perhaps something for the future, once we've cleaned up our project structure (and it's easier to get the info directly from the manifest.py or argparse info?)

Configs
-------

This section of the documentation will show the custom configurations for the individual steps of the tool.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this section should be renamed/removed, now that the 'configs' aren't included here?

@erinhmclark erinhmclark merged commit 113a4db into main Jan 21, 2025
4 checks passed
@erinhmclark erinhmclark deleted the feat/documentation branch January 21, 2025 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants