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 docs generation #33

Merged
merged 24 commits into from
Jun 17, 2024
Merged

Add Sphinx docs generation #33

merged 24 commits into from
Jun 17, 2024

Conversation

Teschl
Copy link
Contributor

@Teschl Teschl commented Jun 6, 2024

This is a first basic implementation of building the docs page with sphinx.

There are still a few issues with this version, so I don't know if we actually want to merge it already. Also, I have not set up a github.io page, that still needs to be added.

  1. All the example .ipynb files have to be located in the /docs folder. There are a few workarounds for this here, they don't really work for .ipynb files though. This issue is has been open for years now.
  2. The generated python docs also have some issues with line breaks, but it seems really inconsistent, so as of right now I haven't been able to solve this.
  3. Even though I am sure that there is no difference in the used theme, the docs for libtopotoolbox still look different in places.

This uses nbsphinx that is installed separately with pip, how do we want to include this in the requirements?

closes #31

@Teschl Teschl marked this pull request as draft June 6, 2024 13:20
@wkearn
Copy link
Contributor

wkearn commented Jun 6, 2024

I've just taken a quick look here. A few initial thoughts.

  1. I ticked the box for GitHub Pages in the settings of this repository, so it should just need a GitHub Actions workflow that builds the docs and uploads them. If you want to test it out, I think you can set up a GitHub Pages site for your fork as well -- that's what I did for testing the libtopotoolbox documentation.
  2. I'm not completely opposed to having all of the example Jupyter notebooks in the docs/ folder (or a docs/examples folder) if that works. Is there a more or less typical way to distribute Jupyter notebooks in Python packages? A user who installs the code from PyPI with pip install won't really have access to them, right?
  3. Can you point to an example where the line break thing happens?
  4. I am pretty sure that this has to do with the versions of sphinx-book-theme. When I install it with pip in my pytopotoolbox environment, it installs v1.1.2, but the packaged Ubuntu version that we install in the Action for libtopotoolbox is v0.1.7. I think it makes sense to update the version in libtopotoolbox (I probably should install it with pip anyway) and stick with the newer look.
  5. To add nbsphinx and sphinx-book-theme and whatever else we might need for the docs, we could add an optional docs dependency like we do with opensimplex, right? Users generally won't need to build the docs themselves, though, so it is okay if installing these dependencies is a bit more complicated for developers.

@Teschl
Copy link
Contributor Author

Teschl commented Jun 6, 2024

Ok, I will get started on this.

Yes, that will work. They are rarely distributed and just kept in the docs. I will move the examples folder.

In the identifyflats docs:
image

I don't think it makes much sense to handle this like we did with opensimplex. Since installing the TopoToolbox package has no direct connection to building the docs. Expanding it's section in the readme could be an option, but I still want to find a way to somehow automate it.

@wkearn
Copy link
Contributor

wkearn commented Jun 6, 2024

I just updated the versions for the libtopotoolbox documentation. I added a requirements.txt file to the docs/ folder there that has the dependencies for building the docs, so that I can run pip install -r docs/requirements.txt to get the correct versions of the dependencies installed. It also means we can pin the versions so we don't get surprised by upstream changes. I'm not sure that's the best way to do it, but it seems to work.

@Teschl
Copy link
Contributor Author

Teschl commented Jun 14, 2024

The structure for the future documentation is now created. See: https://teschl.github.io/pytopotoolbox/index.html. Examples are in a gallery with thumbnails, the API docs are created using autosummary. To make them truly functional, #34 has to be implemented. Then I can also start with actually creating a tutorial, for example.

@wkearn in my opinion this can be merged in this state, since there isn't really any more information to add. At least that I can think of.

@Teschl Teschl marked this pull request as ready for review June 14, 2024 10:13
@wkearn wkearn self-requested a review June 14, 2024 11:01
Copy link
Contributor

@wkearn wkearn left a comment

Choose a reason for hiding this comment

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

Looks great, @Teschl. There are a few things that I think are worth clearing up now (see the comments). The overall framework I think is pretty good. I especially like how the Jupyter notebooks get rendered, and I think that will be useful as we add more documentation.

.github/workflows/docs.yaml Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/installing.rst Outdated Show resolved Hide resolved
@wkearn
Copy link
Contributor

wkearn commented Jun 17, 2024

@Teschl
Copy link
Contributor Author

Teschl commented Jun 17, 2024

I addressed the requested changes @wkearn
Next I will work on refactoring, then the API docs should work as intended and I can get started on writing more examples/tutorials.

@wkearn wkearn merged commit 9e4d9e7 into TopoToolbox:main Jun 17, 2024
3 checks passed
@Teschl Teschl deleted the docs branch June 17, 2024 12:41
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

Successfully merging this pull request may close these issues.

Set up Sphinx + autodoc
2 participants