Skip to content

Gempyor sphinx documentation#550

Merged
emprzy merged 20 commits into
devfrom
gempyor-sphinx-documentation
Apr 28, 2025
Merged

Gempyor sphinx documentation#550
emprzy merged 20 commits into
devfrom
gempyor-sphinx-documentation

Conversation

@emprzy
Copy link
Copy Markdown
Collaborator

@emprzy emprzy commented Apr 16, 2025

Describe your changes.

This pull request contains the framework for generating Sphinx documentation, autosummaries, etc. for the gempyor API (all of its modules except for the ones we wouldn't want public-facing documentation for). It also adds a job in the gempyor-ci GH workflow that fails if users have made changes to the gempyor codebase and not re-generated the Sphinx docs since doing so.

Addressing the downfield: (likely for other PRs)

  • Refurbish the docstrings in gempyor submodules (to be of higher quality). Presently, all those documented with Sphinx have one-sentence blurbs, but many have nothing more than that.
  • Marry this documentation with the gitbook (somehow...)
  • Refine the information we want included in the Sphinx documentation (e.g., remove extraneous blurbs, remove certain funcs. etc.)

Does this pull request make any user interface changes? If so please describe.

Devs can run make fullbuild in the flepiMoP/flepimop/gempyor_pkg/docs directory to generate/re-generate Sphinx documentation.

What does your pull request address? Tag relevant issues.

This pull request addresses GH #458

Tag relevant team members.

@pearsonca @TimothyWillard

emprzy added 2 commits April 16, 2025 10:47
when will I remember to do this the first time through...
Comment thread flepimop/gempyor_pkg/pyproject.toml Outdated
@emprzy
Copy link
Copy Markdown
Collaborator Author

emprzy commented Apr 18, 2025

Question for the group, if I can direct your attention to the index.rst file that helps to configure Sphinx:

I have explicitly specified the submodules of gempyor that would be useful to have public-facing documentation on (by implicitly excluding things like shared_cli.py, cli.py, etc.). Would y'all prefer that I, instead, direct Sphinx to all of gempyor and then explicitly exclude the ones we want to exclude?

Either method requires that we update the .rst file if we add a file that we want documentation on (in the former), or add a file that we don't want documentation on (in the latter). This is pretty nit picky, but I'm trying to make this as dynamic as possible!

@emprzy emprzy marked this pull request as ready for review April 21, 2025 14:04
Change job to ignore `build/` directory changes
@emprzy
Copy link
Copy Markdown
Collaborator Author

emprzy commented Apr 22, 2025

Presently the workflow I implemented runs make fullbuild to generate Sphinx documentation, then checks for differences between this and the files that are being pushed. I have learned that this is a precarious set up, specifically relating to the .rst files, because there are likely to be minute differences in the .rst files created by make fullbuild in the GH workflow that can be due to different Sphinx installation versions, different Python versions, different locations/timestamp metadata, etc., resulting in the workflow failing even if it seems like there aren't any new changes to be committed on the dev's end. There are a couple of solutions I am considering to fix this:

  1. Split the generated .rst files (e.g. generated/*.rst, api/*.rst) into a separate dir and exclude them from the check, but this doesn't necessarily provide as rigorous of a check in the workflow job that Sphinx stuff is up to date
  2. Change the Sphinx job to auto-commit the documentation generation for the user so that they don't have to do it

Copy link
Copy Markdown
Contributor

@TimothyWillard TimothyWillard left a comment

Choose a reason for hiding this comment

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

Looks good to me, added one change related to a warning produced by sphinx because it's an easy fix. Other warnings come from docstrings which should be addressed in follow ups.

I think the first follow up to this should be addressing the gempyor.config_validator module, seems like a helpful reference but as is results in an import error so it's hard to keep in its current state.

Comment thread flepimop/gempyor_pkg/docs/source/conf.py
Comment thread flepimop/gempyor_pkg/docs/source/conf.py
Comment thread flepimop/gempyor_pkg/docs/source/index.rst Outdated
Comment thread flepimop/gempyor_pkg/docs/source/index.rst Outdated
Comment thread .github/workflows/gempyor-ci.yml Outdated
Comment thread .github/workflows/gempyor-ci.yml Outdated
Co-authored-by: Timothy Willard <9395586+TimothyWillard@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@pearsonca pearsonca left a comment

Choose a reason for hiding this comment

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

Seems fine generally, but one question: seems like many of the .rst files are automatically generated / generate-able.

if so, do they belong in the repository?

@emprzy
Copy link
Copy Markdown
Collaborator Author

emprzy commented Apr 25, 2025

Seems fine generally, but one question: seems like many of the .rst files are automatically generated / generate-able.

if so, do they belong in the repository?

Meaning, you think the .rst files should be in the .gitignore? @pearsonca

@pearsonca
Copy link
Copy Markdown
Contributor

Seems fine generally, but one question: seems like many of the .rst files are automatically generated / generate-able.
if so, do they belong in the repository?

Meaning, you think the .rst files should be in the .gitignore? @pearsonca

If they are generated by the build process, and would change whenever we, say, add a function to a module - then yeah, i think the should be .gitignore'd.

Basically, if they are a derived product, then they should be ignored so we don't see multiple files changing due to only one real change.

Add all .rst files to `.gitignore`, make `gempyor` submodule documentation more general, remove unnecessary `BRANCH_NAME` constant
@emprzy emprzy merged commit f4b1083 into dev Apr 28, 2025
8 checks passed
@emprzy emprzy deleted the gempyor-sphinx-documentation branch April 30, 2025 13:16
@TimothyWillard TimothyWillard linked an issue Apr 30, 2025 that may be closed by this pull request
3 tasks
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.

[Feature request]: sphinx documentation for gempyor API

3 participants