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

Test docs on PR; deploy docs on push to default branch #255

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

peytondmurray
Copy link
Collaborator

@peytondmurray peytondmurray commented Jul 6, 2023

This PR handles automatic building and deploying of the docs.

  • A new workflow was added to ensure doc builds pass on every PR
  • A new workflow was added which builds and deploys docs when there's a push to the master branch. This workflow makes use of the new github pages action-based deployment mechanism.
  • Finally, sphinx refused to build unless I used a string type annotation for delete_versions. I think this has something to do with h5py.File being a dependency, but I couldn't get sphinx to build if I did anything else. Here's all the things I tried that failed to build:
    • Using sphinx.ext.intersphinx and setting intersphinx_mapping to the h5py docs. I verified that the correct object file was being pulled by sphinx during the build process, and that the object file contained the File class as expected
    • Using autodoc_mock_modules to mock out the external dependency; still sphinx complained about not being able to find the class definition
    • If I do import h5py inside replay.py the build fails with the string annotation
    • If I do import h5py inside replay.py and convert the string annotation to an actual reference to the File class, the build fails
  • Remove gh-pages publish step from rever.xsh

So while it would be nice to have an intersphinx connection, I'm not sure it's worth it at this time to continue fighting with sphinx. If anyone knows how to do this correctly, a PR against the branch would be welcome.

Closes #199.

Testing

I tested these workflows in a separate repository and was able to deploy the docs successfully.

@peytondmurray peytondmurray marked this pull request as draft July 14, 2023 19:36
@peytondmurray peytondmurray force-pushed the push-gh-pages-on-release branch 3 times, most recently from 4fbba0d to d9fbdfa Compare July 14, 2023 20:21
@peytondmurray peytondmurray marked this pull request as ready for review July 14, 2023 20:25
asmeurer added a commit to asmeurer/versioned-hdf5 that referenced this pull request Jul 27, 2023
@asmeurer
Copy link
Collaborator

Sphinx has an annoying behavior where it tries to create links for type hints. If you can't get it to work, you can probably get it to work with some combination of nitpick_ignore, nitpick_ignore_regex, and autodoc_type_aliases in conf.py. Here's an example that how we had to use those in the array-api repo (in that repo, the type hints are mostly "fake" and can't actually be cross-referenced to anything) https://github.com/data-apis/array-api/blob/main/src/_array_api_conf.py#L52-L77

@asmeurer
Copy link
Collaborator

This looks fine. The only way to really know is to merge it. I would just make sure to check that whatever commit is made to the gh-pages branch once this is merged looks OK (e.g., no unexpected files are added or deleted or anything like that), and that the docs don't break.

@peytondmurray
Copy link
Collaborator Author

Thanks - I'll spend a few more minutes trying to get type hints to work before merging. I should also mention that we'll change the docs deployment mechanism after this merge: instead of committing the static site to the gh-pages branch, we'll just build it in this step and github handles the rest. Once we're confident it's working, we can even get rid of the gh-pages branch.

@asmeurer
Copy link
Collaborator

I'm not so sure about that. gh-pages is how GitHub pages works. Also the benchmarks are also hosted there too.

@peytondmurray
Copy link
Collaborator Author

I know gh-pages used to be the way that pages were hosted. But the action used here (https://github.com/actions/deploy-pages) deploys pages from workflow artifacts, not from a branch. Unless I've misunderstood how this works?

@peytondmurray
Copy link
Collaborator Author

@asmeurer Thanks for the sphinx advice above. I actually got this working just perfectly by

  1. Adding an intersphinx mapping for python and h5py docs
  2. Using autodoc_type_aliases to point File to h5py.File

Now in our docs python or h5py types now point to the python or h5py documentation directly for the functions that have type annotations that use these types. 🎉

@peytondmurray peytondmurray merged commit 07dad97 into deshaw:master Aug 25, 2023
7 checks passed
@peytondmurray peytondmurray deleted the push-gh-pages-on-release branch August 25, 2023 17:43
@peytondmurray peytondmurray mentioned this pull request Sep 1, 2023
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.

Fix automatic docs updating on CI
2 participants