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

Replace Cartopy with PyGMT #327

Merged
merged 23 commits into from
Sep 9, 2022
Merged

Conversation

mdtanker
Copy link
Member

@mdtanker mdtanker commented Jun 20, 2022

Description

Update Gallery and User Guide to use PyGMT for plotting instead of Cartopy.
Ditch Cartopy from the requirements-*.txt files and from environment.yml
and add GMT and PyGMT. Pin GMT, PyGMT and gdal versions to avoid failures
during doc building. The versions were obtained from trial and error on local
system, inspired on the versions working on GitHub Actions. Edit doc/Makefile
to disable PyGMT usage of external display when building the docs. Pin Python
to 3.9.* to avoid strictly pinning Python 3.9.0.

I tried to match the original example plots as best as I could.

Relevant issues/PRs

Fixes #317

@mdtanker mdtanker changed the title Pygmt examples Replace Cartopy with PyGMT Jun 20, 2022
@mdtanker
Copy link
Member Author

A change to EquivalentSources which I didn't have included in my environment is causing errors with a few of the examples, I'll try and fix this now.

@mdtanker
Copy link
Member Author

I'm happy to update the User Guide examples as well. I think the ones I've already committed were for the old user guide example in the data folder. How do I update .rst files for the new user guide examples? For example the user_guide/gravity_disturbance.rst file?

@leouieda
Copy link
Member

@mdtanker with the new structure @santisoler built, you can edit those directly. The jupyter-execute blocks are run by Sphinx when building the website and outputs get included automatically. So you don't have to edit .py files anymore for the documentation.

@mdtanker
Copy link
Member Author

@leouieda how can I run the .rst files to see my changes? I'm trying to build the docs with cd docs and make all but the process keeps getting aborted with make: *** [Makefile:45: html] Aborted (core dumped). Both folders doc/_build/doctrees and doc/_build/html are empty.

@mdtanker
Copy link
Member Author

I've now updated the user_guide/forward_modelling examples. I'm able to create the docs now, but all the gallery examples have a broken image, and the plots aren't showing, even though the plotting scripts run without issues on my computer. The user_guide docs appear to be correct. I'll update the user_guide/equivalent_sources to pygmt soon.

@leouieda
Copy link
Member

leouieda commented Jun 28, 2022

@mdtanker sorry I forgot to say that sphinx-gallery needs to be configured to capture PyGMT figures. See this line in the doc/conf.py of Ensaio https:/fatiando/ensaio/blob/24d3910c807e0dc3fa6fd5d356609d97b56cd74c/doc/conf.py#L99

There's also an import Sat there top that needs to happen as well.

@mdtanker
Copy link
Member Author

mdtanker commented Jul 4, 2022

@leouieda thanks, I think that's fixed it!

I've converted all of the figures in the .rst files in the user guide from matplotlib to pygmt. I skipped doc/user_guide/gravity_disturbance.rst as per fatiando/boule#31.

I've also converted the gallery examples in doc/gallery.

Before I realized they aren't being used anymore, I converted the .py files under data/examples.

I'm happy to change the styling of the plots, like projection, font size, frame type, etc.

@mdtanker
Copy link
Member Author

mdtanker commented Jul 4, 2022

Not sure why the documentation build has failed, it built fine on my local machine...

@leouieda
Copy link
Member

@mdtanker it looks like it timed out when downloading the sample data. I'm rerunning it to see it was an intermittent problem.

Thanks for all of this!

@leouieda
Copy link
Member

Found the culprit! PyGMT will open the figure and wait for it to be closed before continuing. It probably does that when you build locally right? That's why it hangs on the CI since the figure never closes.

To prevent that, the doc/Makefile needs to be modified to look like this:
https:/fatiando/ensaio/blob/main/doc/Makefile#L43

@mdtanker
Copy link
Member Author

Ah great, thanks for finding that! I'll fix that now and try again. I noticed in the ensaio .yml (link below) that GMT is included, as well as version requirements for both GMT and pyGMT. Should I add GMT specifically to the harmonica .yml? For consistency, should I use the same versions as ensaio?

  • pygmt==0.5.0
  • gmt==6.2.0

https:/fatiando/ensaio/blob/24d3910c807e0dc3fa6fd5d356609d97b56cd74c/environment.yml#L28

@mdtanker
Copy link
Member Author

I have not removed cartopy or matplotlib from the following places, but happy to do so if it's appropriate:

env/requirements-docs.txt
doc/conf.py
environment.yml

I also noticed in init.py there is a test for whether generated figures match against baseline figures. Looking at the PyGMT contributors guide looks like it works for comparing PyGMT plots as well.

@leouieda do we want to implement these figure tests in Harmonica?

@santisoler santisoler added this to the v0.6.0 milestone Aug 12, 2022
@mdtanker
Copy link
Member Author

@santisoler let me know if there's anything else you want me to do for these examples to be able to merge them!

@santisoler
Copy link
Member

Hi @mdtanker! Sorry for leaving this PR a little bit behind. I'll take a look at it, try to resolve the conflicts we have now and I'll come back if it needs something else in order to be merged.

Do you consider it's ready? Has all the examples been ported to PyGMT?

@mdtanker
Copy link
Member Author

No worries and no rush. Yes, I believe all the User Guide and Gallery examples have been converted, except for doc/user_guide/gravity_disturbance.rst since there was discussion of removing it from Harmonica fatiando/boule#31.

I think the three conflicts are from #342, but I had already added vd.get_region to those three examples. Let me know if there are any figure style changes you want or if there's anything that's not working!

@mdtanker
Copy link
Member Author

mdtanker commented Sep 2, 2022

I've resolved the conflicts and I think it's all ready to merge. Here's a pdf of the gallery index page.

Gallery Harmonica dev.pdf

The docs built fine on my local machine, but I'm not sure how to check if they're built alright in the PR.

Pin versions of pygmt, gmt and gdal in environment.yml and
env/requirements-docs.txt. Also pin `python==3.9.*` to allow installing
newer versions that python 3.9.0. The gdal version was pinned based on
trial and error after facing issues when trying to build the docs
locally in my machine.
@santisoler
Copy link
Member

I was facing some issues when trying to build the docs locally with the packages installed through the environment.yml. Apparently the problem was related to the gdal version. Since CIs were building without any issues with gdal==3.5.0, I tried that one and it worked. Nevertheless, I had to change the pinned version of Python, because it was pinning 3.9.0 what through errors when trying to solve the dependency tree.

With this environment.yml file it's working. And I've also pinned GMT just in case. @leouieda would you check if it's working locally for you as well?

@santisoler
Copy link
Member

@mdtanker I'm merging this. I think this is more than ready. If we spot any detail we want to change, we can do so in another PR. Thank you very much for this, tremendous work! 🏅🚀

@santisoler santisoler merged commit 0ec2665 into fatiando:main Sep 9, 2022
@mdtanker mdtanker deleted the pygmt_examples branch September 10, 2022 17:44
@mdtanker
Copy link
Member Author

mdtanker commented Sep 10, 2022

Thanks @santisoler! I just noticed that even though the User Guide .rst files (point.rst for example) have been converted to PyGMT, the docs still show the old cartopy version (Forward Modeling - Points). If you switch to the dev version of the docs then they have updated to PyGMT (https://www.fatiando.org/harmonica/dev/user_guide/forward_modelling/point.html).

Any idea why the Gallery examples have been updated, but the .rst User Guides haven't been updated?

Same thing seems to have happened to the install.rst file, where cartopy should have been replaced by pygmt.

@santisoler
Copy link
Member

@mdtanker The Forward Modeling - Points page you are looking at corresponds to the docs for the latest version (Harmonica v0.5.1). Although this PR have already been merged, these changes could be seen only in the dev docs until we make another release. I'm planning on releasing v0.6.0 way sooner that we did with v0.5.0. It's better to release more often after small incremental changes than having a huge release that sits for almost a year.

@mdtanker
Copy link
Member Author

Okay that makes sense, I thought it was part of v.0.5.1 but just wasn't updating 👍

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.

Replace cartopy with pygmt in docs
3 participants