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

Migrate Continuous Documentation from Vercel to Readthedocs #1859

Merged
merged 16 commits into from
Jul 31, 2022

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Mar 31, 2022

Description of proposed changes

PyGMT has been using Vercel (previously Zeit Now) since 2019, and that's been a great tool in checking out documentation previews on individual Pull Requests (PRs). However, the legacy service plan we're on will be deprecated on 12 May 2022, see https://vercel.com/support/articles/legacy-plans-deprecation

This Pull Request migrates the configuration to ReadtheDocs which is another web hosting platform. Note that this will only affect the redendered documentation previews for PRs. The documentation built on the main branch is still built and deployed using GitHub Actions.

I've set up the new URL to point to https://pygmt-devs.readthedocs.io. @GenericMappingTools/pygmt-maintainers, for those of you who would like access to the readthedocs backend, please register for an account on https://readthedocs.org/accounts/signup, and ping me so that I can add you to the maintainer list.

Cc @meghanrjones @seisman, I might need help setting up the readthedocs webhooks integration on the pygmt repo (ref https://docs.readthedocs.io/en/stable/integrations.html)

References:

Supersedes #344

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@weiji14 weiji14 added the maintenance Boring but important stuff for the core devs label Mar 31, 2022
@weiji14 weiji14 added this to the 0.6.1 milestone Mar 31, 2022
@weiji14 weiji14 self-assigned this Mar 31, 2022
@vercel
Copy link

vercel bot commented Mar 31, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/gmt/pygmt/GMATHSZtsYyH4oV2uCrzxwMs9Uxh
✅ Preview: https://pygmt-git-verceltoreadthedocs-gmt.vercel.app

@seisman
Copy link
Member

seisman commented Apr 1, 2022

Cc @meghanrjones @seisman, I might need help setting up the readthedocs webhooks integration on the pygmt repo (ref docs.readthedocs.io/en/stable/integrations.html)

Approved.

@seisman
Copy link
Member

seisman commented Apr 1, 2022

This Pull Request migrates the configuration to Read_the_Docs which is another web hosting platform. Note that this will only affect the redendered documentation previews for PRs. The documentation built on the main branch is still built and deployed using GitHub Actions. I've set up the new URL to point to pygmt-devs.readthedocs.io.

My concern is, we will have two domains that host the exact same contents, which may affect the SEO of of main site.

@weiji14
Copy link
Member Author

weiji14 commented Apr 3, 2022

This Pull Request migrates the configuration to Read_the_Docs which is another web hosting platform. Note that this will only affect the redendered documentation previews for PRs. The documentation built on the main branch is still built and deployed using GitHub Actions. I've set up the new URL to point to pygmt-devs.readthedocs.io.

My concern is, we will have two domains that host the exact same contents, which may affect the SEO of of main site.

I've set the canonical URL to point to https://pygmt.org following https://docs.readthedocs.io/en/stable/custom_domains.html#canonical-urls. I think that should ensure that search engines point to the existing .org site?

@seisman
Copy link
Member

seisman commented Apr 3, 2022

This Pull Request migrates the configuration to Read_the_Docs which is another web hosting platform. Note that this will only affect the redendered documentation previews for PRs. The documentation built on the main branch is still built and deployed using GitHub Actions. I've set up the new URL to point to pygmt-devs.readthedocs.io.

My concern is, we will have two domains that host the exact same contents, which may affect the SEO of of main site.

I've set the canonical URL to point to pygmt.org following docs.readthedocs.io/en/stable/custom_domains.html#canonical-urls. I think that should ensure that search engines point to the existing .org site?

Looks great.

@weiji14
Copy link
Member Author

weiji14 commented Apr 3, 2022

This Pull Request migrates the configuration to Read_the_Docs which is another web hosting platform. Note that this will only affect the redendered documentation previews for PRs. The documentation built on the main branch is still built and deployed using GitHub Actions. I've set up the new URL to point to pygmt-devs.readthedocs.io.

My concern is, we will have two domains that host the exact same contents, which may affect the SEO of of main site.

I've set the canonical URL to point to pygmt.org following docs.readthedocs.io/en/stable/custom_domains.html#canonical-urls. I think that should ensure that search engines point to the existing .org site?

Looks great.

Hmm, we might need to modify the pygmt.org DNS record somehow... Now the readthedocs preview documentation link points to http://pygmt.org/en/vercel_to_readthedocs 😅

@seisman seisman modified the milestones: 0.6.1, 0.7.0 Apr 9, 2022
@weiji14
Copy link
Member Author

weiji14 commented Jun 10, 2022

I'm going to be flat out busy for the next month (travel, workshops, etc), so I probably won't have time to finish this up. @GenericMappingTools/pygmt-maintainers can someone maybe take over this Vercel to Readthedocs migration PR since Vercel is expiring soon? If you sign up for a readthedocs account at https://readthedocs.org/accounts/login/, I can add you to the pygmt-dev maintainer list at https://readthedocs.org/dashboard/pygmt-dev/users

image

@seisman
Copy link
Member

seisman commented Jun 11, 2022

Please add me as a maintainer so that I can give it a try.

@weiji14
Copy link
Member Author

weiji14 commented Jun 11, 2022

Please add me as a maintainer so that I can give it a try.

Ok, done.

@seisman
Copy link
Member

seisman commented Jun 15, 2022

This Pull Request migrates the configuration to Read_the_Docs which is another web hosting platform. Note that this will only affect the redendered documentation previews for PRs. The documentation built on the main branch is still built and deployed using GitHub Actions. I've set up the new URL to point to pygmt-devs.readthedocs.io.

My concern is, we will have two domains that host the exact same contents, which may affect the SEO of of main site.

I've set the canonical URL to point to pygmt.org following docs.readthedocs.io/en/stable/custom_domains.html#canonical-urls. I think that should ensure that search engines point to the existing .org site?

Looks great.

Hmm, we might need to modify the pygmt.org DNS record somehow... Now the readthedocs preview documentation link points to http://pygmt.org/en/vercel_to_readthedocs 😅

I think you were NOT reading the right documentation when setting the canonical URLs. The correct documentation is here: https://docs.readthedocs.io/en/stable/canonical-urls.html. I've removed the settings of custom domains in the Admin panel.

@vercel
Copy link

vercel bot commented Jun 15, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
pygmt ✅ Ready (Inspect) Visit Preview Jul 30, 2022 at 1:49AM (UTC)

@seisman seisman modified the milestones: 0.7.0, 0.8.0 Jun 19, 2022

# Optionally declare the Python requirements required to build your docs
conda:
environment: environment.yml
Copy link
Member Author

@weiji14 weiji14 Jun 22, 2022

Choose a reason for hiding this comment

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

Do you think we should just install the sphinx-related libraries needed for building the docs? We used to install everything on Vercel because it's quite fast, but Readthedocs is a bit slower it seems, so less libraries might speed things up.

Refer to ci_docs.yml for dependencies needed:

mamba install gmt=6.3.0 numpy pandas xarray netCDF4 packaging \
build ipython make myst-parser geopandas \
sphinx sphinx-copybutton sphinx-design sphinx-gallery sphinx_rtd_theme

Copy link
Member

Choose a reason for hiding this comment

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

Done in d21acf9.

Slightly faster. Now it takes 12 minutes.

@@ -0,0 +1,26 @@
name: pygmt
Copy link
Member

Choose a reason for hiding this comment

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

Is .github/workflows a good place to save these environment files?

I believe we will also use it in ci_docs.yml.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Sounds a good place to put all CI related files.

Ping @weiji14 for comments on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally we would put all these 'docs' dependencies as an 'extras' in pyproject.toml/setup.py following https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optional-dependencies so that we could just pip install .[docs]. However, GMT would still need to come from conda-forge, unless #1853 becomes a thing 🙂

Copy link
Member Author

@weiji14 weiji14 Jun 23, 2022

Choose a reason for hiding this comment

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

Actually, is the GMT C library needed to build the docs? Could we just mock it out? I recall @meghanrjones was able to use https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#confval-autodoc_mock_imports at xarray-contrib/xbatcher#62 (comment), though not sure if it works in this case. If it does work though, then my comment at https://github.com/GenericMappingTools/pygmt/pull/1859/files#r904531125 about using 'extras' would be the way to go.

Copy link
Member

@seisman seisman Jun 23, 2022

Choose a reason for hiding this comment

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

Actually, is the GMT C library needed to build the docs? Could we just mock it out?

I don't think it's possible. We need to call the GMT C library to generate images (i.e.., all the images generated by sphinx-gallery).

dependencies:
# Required dependencies
- pip
- gmt=6.3.0
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

@weiji14 weiji14 Jul 11, 2022

Choose a reason for hiding this comment

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

Yeah, if we merge this PR first, will need to update bump_gmt_checklist.md directly in #1990 later.

@seisman seisman marked this pull request as ready for review July 11, 2022 09:43
@seisman seisman added the needs review This PR has higher priority and needs review. label Jul 11, 2022
Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! We should update the permissions for the vercel app after merging this.

@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Jul 30, 2022
@seisman seisman merged commit d3b5fdc into main Jul 31, 2022
@seisman seisman deleted the vercel_to_readthedocs branch July 31, 2022 05:27
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jul 31, 2022
@weiji14
Copy link
Member Author

weiji14 commented Jul 31, 2022

Just wanted to say, thanks @maxrjones and @seisman for keeping things going! Sorry that I haven't been as active lately, it's been really intense with FDL2022 (even over weekends), and I'll probably be busy until mid-August at least. Hoping to get back up to speed on PyGMT contributions from September onwards after I catch up with a backlog of stuff!

LGTM, thanks! We should update the permissions for the vercel app after merging this.

I took at a look at the Vercel project dashboard for pygmt, and it should be disconnected already as far as I can tell (maybe someone already disconnected it in the GitHub org settings)? I didn't delete the PyGMT project on Vercel, so the old links vercel links in PRs should still work as long as they're still willing to host it. Will keep an eye out in case comments like d3b5fdc#commitcomment-79850583 still pop up.

@seisman
Copy link
Member

seisman commented Jul 31, 2022

I took at a look at the Vercel project dashboard for pygmt, and it should be disconnected already as far as I can tell (maybe someone already disconnected it in the GitHub org settings)?

Yes, I did it.

Will keep an eye out in case comments like d3b5fdc#commitcomment-79850583 still pop up.

I think the comment was made before I disconnected the Vercel service, so we should be good to go.

sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…appingTools#1859)

* Remove vercel configuration files
* Setup readthedocs configuration file
* Run pip install . in the readthedocs build environment
* Remove vercel configurations and add readthedocs configurations in MANIFEST.in
* Update doc/maintenance.md to trigger builds in PR
* Set canonical URL by setting html_baseurl
* Update contributing and maintenance guides
* Customize the pre_build step to generate API stub files
* Only install packages required for building docs
* Move the docs environment file to the ci directory

Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants