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

Support for multi version documentation #3862

Merged
merged 53 commits into from
Apr 16, 2023
Merged

Conversation

AlejandroFernandezLuces
Copy link
Contributor

@AlejandroFernandezLuces AlejandroFernandezLuces commented Jan 23, 2023

Overview

Adds support for multi version documentation by using new actions developed in PyAnsys. With this feature we can have several versions of the documentation in the same domain. Right now, this PR only uploads the multiversion docs to github pages and keeps the current documentation as is, so we don't mess with the current documentation. Once this PR is discussed, we can open another PR to remove the actions related to the old documentation. Closes #3835

Details

  • Allows both multiversion and single version documentation automatic generation and upload.
  • Requires to work with two different folder structure, so we need to upload and download the same documentation twice. This should be solved once we settle on the multiversion docs.

References

PyAnsys multi version documentation actions examples

PyAnsys multi version documentation docs

@github-actions github-actions bot added documentation Anything related to the documentation/website maintenance Low-impact maintenance activity labels Jan 23, 2023
@codecov
Copy link

codecov bot commented Jan 23, 2023

Codecov Report

Merging #3862 (9e9b222) into main (b12e45a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3862   +/-   ##
=======================================
  Coverage   95.77%   95.77%           
=======================================
  Files          97       97           
  Lines       20755    20755           
=======================================
  Hits        19879    19879           
  Misses        876      876           

Copy link
Contributor

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

This is going in the right direction. Just left some minor comments:

  • Declare the DOCUMENTATION_CNAME at the very top of the CI/CD pipeline as an environment variable. See other PyAnsys projects using multi-version.

  • Make sure you use the doc-artifact-name to indicate the doc-deploy-* which artifact is the one containing the documentation of the project.

.github/workflows/docs.yml Outdated Show resolved Hide resolved
doc/conf.py Outdated Show resolved Hide resolved
.github/workflows/docs.yml Show resolved Hide resolved
.github/workflows/docs.yml Show resolved Hide resolved
.github/workflows/docs.yml Show resolved Hide resolved
Copy link
Contributor

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

This is looking good, @AlejandroFernandezLuces.

Make sure that the cname in this versions.json file is the right one. The domain should be pyvista.org not pyvista.

Also, I think the documentation is failing because pydata-sphinx-theme complains about the versions.json file not existing in the current CNAME for the documentation. I bet this is because the latest versions of the pydata-sphinx-theme check for this file.

A trick to avoid this may be to place an empty file versions.json file in the documentation repositories and before publishing new docs to those repos. This way, documetation build will pass and it won't affect the future CI/CDs.

Once the gh-pages are cluttered, the PyVista maintainers will take the decission to activate those.

.github/workflows/docs.yml Outdated Show resolved Hide resolved
@jorgepiloto
Copy link
Contributor

My previous approach was a bit complicated. Let's see if we can ignore that link check in the conf.py.

@jorgepiloto
Copy link
Contributor

Try to add this in the configuartion for the theme: https://pydata-sphinx-theme.readthedocs.io/en/stable/user_guide/version-dropdown.html#configure-switcher-json-url

html_theme_options = {
    # ...
    "check_switcher": False
}

@jorgepiloto
Copy link
Contributor

It turns out this was the reason for making the docs pass. However, this failed silently. Take a look to the logs. The workflow did not found the artifact.

@AlejandroFernandezLuces
Copy link
Contributor Author

It turns out this was the reason for making the docs pass. However, this failed silently. Take a look to the logs. The workflow did not found the artifact.

Yes, also I saw earlier that the gh-pages branch is empty. There is probably some error on how I managed the paths.

@banesullivan
Copy link
Member

Yes, also I saw earlier that the gh-pages branch is empty. There is probably some error on how I managed the paths.

We DO NOT deploy to the github pages branch of this repo as it will blow up the repo size very quickly. Deployed documentation is hosted in separate repos for this reason:

@banesullivan
Copy link
Member

@AlejandroFernandezLuces, I am going to delete the gh-pages branch added from this work (see d0a5a25). Please make sure to push to a separate repository. In fact, please do NOT push to the official repos but to a separate, isolated repo while testing. I made one for you at: https://github.com/pyvista/multiversion-docs-testing

@jorgepiloto
Copy link
Contributor

Thanks for pointing this out, @banesullivan. We need to modify the pyansys/actions so the multi-version allows to deploy to a third repository. It should be quick to implement, though.

@AlejandroFernandezLuces
Copy link
Contributor Author

@AlejandroFernandezLuces, I am going to delete the gh-pages branch added from this work (see d0a5a25). Please make sure to push to a separate repository. In fact, please do NOT push to the official repos but to a separate, isolated repo while testing. I made one for you at: https://github.com/pyvista/multiversion-docs-testing

Thanks for creating the repo! We weren't aware that was the reason pyvista doesn't have a gh-pages itself. We'll do the adaptations needed for deploying in other repos and we'll deploy on the new repo.

@banesullivan
Copy link
Member

FYI, while deleting the gh-pages branch here I noticed it had a shared history with main... you may want to make sure the actions your are making create an orphan branch for gh-pages as it should definitely not share a history with the source branch.

FWIW, we currently use https://github.com/peaceiris/actions-gh-pages which I believe does a good job of this... I'd recommend using that action for the deploy step rather than any custom logic

@banesullivan
Copy link
Member

Indeed, the actions you are making are already composite actions... I'd highly recommend switching this:

https://github.com/pyansys/actions/blob/4ecfff6332d68ed17cf338cc7ea66e77b376e1fd/doc-deploy-stable/action.yml#L153

to use https://github.com/peaceiris/actions-gh-pages

Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

Really excited about the progress with this.

For now, set the target external repo to https://github.com/pyvista/pyvista-docs-dev so we can debug this reasonably.

I think deploying to a separate repository should always be an option, namely being that we don't want to bloat a repo with 100s of MB of documentation. I think the mne docs are a great example with why you'd like to do this. If we're going to track multiple versions, it will get big quickly, especially with something like pyvista where we have a zillion images.

.github/workflows/docs.yml Outdated Show resolved Hide resolved
@banesullivan
Copy link
Member

@akaszynski, I left a comment earlier here that they should use https://github.com/pyvista/multiversion-docs-testing for this PR so as not to affect the documentation used in production.

I want to extra emphasize to please use https://github.com/pyvista/multiversion-docs-testing and NOT https://github.com/pyvista/pyvista-docs-dev until this PR is finished

@jorgepiloto
Copy link
Contributor

The great discussion that took place here made us to rethink about the structure and workflows for generating multi-version documentation. Our final thoughts got collected in this discussion.

@jorgepiloto
Copy link
Contributor

PyAnsys actions v4 is out, see https://actions.docs.pyansys.com/version/stable/

Now, it is possible to deploy to an external repository, desired branch and forcing this branch to be an orphan one.

Pinging here @AlejandroFernandezLuces.

.gitignore Outdated Show resolved Hide resolved
doc/source/conf.py Outdated Show resolved Hide resolved
requirements_docs.txt Outdated Show resolved Hide resolved
@akaszynski
Copy link
Member

@AlejandroFernandezLuces, thanks for your work here. I think this is finally good to go.

Just so everyone knows, I've already switched out docs to use the multi-version at https://docs.pyvista.org/. This was tested at 0500 UTC, Saturday. It appears that no PyVisters were harmed in the process and the multi-version switcher actually works:
tmp


Before making the switch, I was worried about redirects from google. For example:
https://docs.pyvista.org/api/core/_autosummary/pyvista.PolyDataFilters.strip.html

This needs to be redirected to:
https://docs.pyvista.org/version/stable/api/core/_autosummary/pyvista.PolyDataFilters.strip.html

There isn't really an existing solution for this since the directory structure of our documentation precludes using something like sphinx-contrib/redirects. Turns out, there's a fairly easy solution: simply generate one html redirect file for each of the existing ones in version/stable. This was done in:
https://github.com/pyvista/pyvista-docs/blob/gh-pages-multiversion/gen-redirect.py


Bonus (because there always is)

We don't do a good job telling Google what it can and can't crawl. We don't even provide a sitemap. While working on the redirects I realized that we should do both. I've added a sitemap.xml, which is generated from [gen-sitemap.py(https://github.com/pyvista/pyvista-docs/blob/gh-pages-multiversion/gen-sitemap.py), as well as robots.txt.

I've added the sitemap to https://search.google.com/search-console/ in hopes that all pages on Google will go to their redirects.

Copy link
Member

@banesullivan banesullivan left a comment

Choose a reason for hiding this comment

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

Awesome, awesome work, all! Excited about this! I don't have the bandwidth to thoroughly dive into/review this but have been casually following along, and fell confident in this merging

Though we may want to address this first as it seems like a new issue from the switch: #4283

Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

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

Sorry to demoralize you, but I noticed the following thing while using the version switch feature. It works on the top page as shown in this video, but the switch tab does not work on other pages. Is this just my environment?

out.mp4

@akaszynski
Copy link
Member

Sorry to demoralize you, but I noticed the following thing while using the version switch feature. It works on the top page as shown in this video, but the switch tab does not work on other pages. Is this just my environment?

Fixed in 2246a0a. This was already fixed in this branch and backported to the older doc generation. Any new docs generated (after merging this PR) will not have this issue.

Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

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

Thanks

@tkoyama010 tkoyama010 merged commit ec78a81 into main Apr 16, 2023
22 checks passed
@tkoyama010 tkoyama010 deleted the doc/multi-version-docs branch April 16, 2023 21:33
@tkoyama010
Copy link
Member

@AlejandroFernandezLuces Thanks a lot. This PR is a great improvement!

@AlejandroFernandezLuces
Copy link
Contributor Author

Thanks everyone for your feedback! I hope PyVista users will find this useful 😄

@adeak
Copy link
Member

adeak commented Apr 17, 2023

I hope PyVista users will find this useful

Considering how dynamically PyVista changes, I think this is super useful to end users. Anyone constrained to old versions can just look up the corresponding docs 👌

@jorgepiloto
Copy link
Contributor

Glad to see this feature got finally merged, @AlejandroFernandezLuces. Congratulations! 🚀

@akaszynski akaszynski mentioned this pull request Apr 30, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Anything related to the documentation/website full-doc Run full documentation build on CI maintenance Low-impact maintenance activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable multi-version documentation
7 participants