harmonica icon indicating copy to clipboard operation
harmonica copied to clipboard

Replace Cartopy with PyGMT

Open mdtanker opened this issue 2 years ago • 12 comments

Description

Updated the Gallery and User Guide examples to use PyGMT for plotting. I tried to match the original example plots as best as I could.

Relevant issues/PRs

Fixes https://github.com/fatiando/harmonica/issues/317

mdtanker avatar Jun 20 '22 03:06 mdtanker

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 avatar Jun 21 '22 01:06 mdtanker

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?

mdtanker avatar Jun 21 '22 03:06 mdtanker

@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.

leouieda avatar Jun 21 '22 08:06 leouieda

@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 avatar Jun 21 '22 21:06 mdtanker

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.

mdtanker avatar Jun 24 '22 04:06 mdtanker

@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://github.com/fatiando/ensaio/blob/24d3910c807e0dc3fa6fd5d356609d97b56cd74c/doc/conf.py#L99

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

leouieda avatar Jun 28 '22 06:06 leouieda

@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 https://github.com/fatiando/boule/issues/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 avatar Jul 04 '22 04:07 mdtanker

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

mdtanker avatar Jul 04 '22 09:07 mdtanker

@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 avatar Jul 17 '22 21:07 leouieda

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://github.com/fatiando/ensaio/blob/main/doc/Makefile#L43

leouieda avatar Jul 17 '22 22:07 leouieda

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://github.com/fatiando/ensaio/blob/24d3910c807e0dc3fa6fd5d356609d97b56cd74c/environment.yml#L28

mdtanker avatar Jul 18 '22 20:07 mdtanker

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?

mdtanker avatar Jul 18 '22 21:07 mdtanker

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

mdtanker avatar Aug 26 '22 19:08 mdtanker

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?

santisoler avatar Aug 26 '22 21:08 santisoler

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 https://github.com/fatiando/boule/issues/31.

I think the three conflicts are from https://github.com/fatiando/harmonica/pull/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 avatar Aug 29 '22 19:08 mdtanker

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.

mdtanker avatar Sep 02 '22 18:09 mdtanker

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 avatar Sep 02 '22 23:09 santisoler

@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 avatar Sep 09 '22 17:09 santisoler

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.

mdtanker avatar Sep 10 '22 20:09 mdtanker

@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.

santisoler avatar Sep 12 '22 16:09 santisoler

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

mdtanker avatar Sep 13 '22 02:09 mdtanker