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

Move docs to docstrings and generate documentation with sphinx-autodoc #367

Merged
merged 5 commits into from
Oct 15, 2024

Conversation

Priyansh121096
Copy link
Contributor

Closes #364

To test, checkout this PR locally, setup you dev env and do cd doc/ && make clean && make html and open doc/build/html/api.html in a browser.

image

@Priyansh121096
Copy link
Contributor Author

CC @WillAyd ^

from typing import List

sys.path.insert(0, os.path.abspath(os.path.join("..", "..", "src")))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to do this without having to change sys.path? Also, I think it would be better if you used a path relative to this file instead of the current working directory of the the shell, so something like:

srcdir = pathlib.Path(__file__).resolve().parent.parent / "src"
# do whatever needs to be done with srcdir

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WillAyd there's two standard ways to work with autodoc as per the official docs.

  1. Do an install of the package in your env and then run sphinx.
  2. Add the sources to the path and then run sphinx.

The latter just seemed less frictional to me as an outside contributor and hence I went ahead with it. I'm not sure if there's a CI workflow to publish the docs or if they're automatically generated. If it's the former and I can assume that the package will be installed in the environment before the docs are generated, I can remove the sys.path.insert. Both workflows are perfectly standard however.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if there's a CI workflow to publish the docs or if they're automatically generated

This is all handled by readthedocs.com, which looks for changes to this repo and republishes the docs when pushes are made to main. If you sign up for an account there I'd be happy to add you to the project; its probably helpful for what you are trying to do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. "Priyansh121096" is my readthedocs username.

@WillAyd
Copy link
Collaborator

WillAyd commented Oct 14, 2024

@Priyansh121096 this looks pretty good. Any improvements to the path handling can be done in a follow up, as you have time/interest.

For now do you mind merging in or rebasing against main? That should fix the CI failures

@Priyansh121096
Copy link
Contributor Author

@WillAyd merged in the changes from main.

@WillAyd WillAyd merged commit 9a18d6c into innobi:main Oct 15, 2024
10 checks passed
@WillAyd
Copy link
Collaborator

WillAyd commented Oct 15, 2024

Thanks @Priyansh121096

@Priyansh121096 Priyansh121096 deleted the fix-docs branch October 15, 2024 12:50
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.

[Docs] Move API documentation from api.rst to docstrings
2 participants