Tutorial 3 - Add first draft of an xarray integration tutorial

[maxrjones]

Preview:

• https://github.com/GenericMappingTools/agu24workshop/blob/add-xarray-tutorial/book/tut03_spec_xarray.ipynb
• https://gitnotebooks.com/GenericMappingTools/agu24workshop/pull/15

[weiji14]

Thanks @maxrjones! Let us know if you're intending to add anything else to this notebook otherwise it looks pretty good already! Was glad to see the CMIP6 Zarr example on sea level change at the end, I made that tutorial back in xarray-contrib/xarray-tutorial#132 during a Scipy sprint, so funny to see it recreated in PyGMT here 😆

Just on the Jupyter cell outputs, do you think we should keep the cell outputs rendered, or clear the outputs and have CI render the cells (depends on how long it takes to pull the NetCDF/Zarr datasets)? If we keep the outputs, maybe we can reduce the size of the plots (change it from 12cm to a lower number)?

[weiji14]

Was trying to put inline comments with gitnotebooks but hit into this error - #9 (comment), so will have to put these all in one go:

1. Description in first few cells

This tutorial is part of the AGU2024 annual meeting GMT/PyGMT pre-conference workshop (PREWS9) Mastering Geospatial Visualizations with GMT/PyGMT

Conference: https://agu.confex.com/agu/agu24/meetingapp.cgi/Session/226736
GitHub: https://github.com/GenericMappingTools/agu24workshop
Website: https://www.generic-mapping-tools.org/agu24workshop
Recommended version: PyGMT v0.13.0 with GMT 6.5.0

Debating on whether all of this needs to be here (thinking of when this goes into the Jupyter Book)? I could put some of this into #12 perhaps?

2. Typo grd_image - grdimage

[yvonnefroehlich]

Was trying to put inline comments with gitnotebooks but hit into this error - #9 (comment)

Please see my comments in Issue #9.

1. Description in first few cells
   Debating on whether all of this needs to be here (thinking of when this goes into the Jupyter Book)? I could put > some of this into Add getting started and installation instructions to the welcome page #12 perhaps?

Yeah, I was thinking about this, when adding this part at the beginning of tutorial 4. I like to see, that all JN or files share a consistent layout of the introduction cell and clearly state to what this JN or script belongs and for which version it should work. So, I feel it makes sense to add this part for all JN or files together within one PR. Do we plan to upload the final workshop to Zenodo? Then we can also add this as a new line later.

[weiji14]

Oh, and just on this pre-commit config, there's this comment at https://github.com/astral-sh/ruff-pre-commit/tree/v0.7.4?tab=readme-ov-file#using-ruff-with-pre-commit:

When running with --fix, Ruff's lint hook should be placed before Ruff's formatter hook, and before Black, isort, and other formatting tools, as Ruff's fix behavior can output code changes that require reformatting

So maybe best to put ruff-pre-commit before black? Or just rely on ruff-format and remove black.

[Esteban82]

Thanks @maxrjones! I think it is very good. Just two minor comments.

1. When you plot the world map, I would add a note explaining that we use Winkel Tripel projection (R) which shows the whole world.
2. Maybe we could set the GMT_DATA_SERVER to NOAA (instead of the default Oceania) to speed up the downloading process.

[yvonnefroehlich]

This JN looks really structured, covers nice aspects and adds some not-seismology, but meteorology / oceanography-related content @maxrjones 🙂!

As already mentioned by @weiji14 reviewing via gitnotebooks does not work here, so I will try to write down my comments and suggestions as a bullet point list:

• Empty cell just as the first cell of the tutorial: Remove it?
• Title: Would capitalize Xarray also here.
• Headings: Maybe add numbers to make it easier to refer to the different parts of the tutorial.
• Resolution of images within the JN: Maybe reduce the resolution to 150 dpi to keep the size of the JN small.
• Section "What is Xarray": Great, that you considered this general part here! Totally, forgot about this in my pandas and GeoPandas tutorial - that there are maybe users who currently do not use (some of) these packages.
• Section "Why use Xarray with PyGMT": Maybe add links to Dask, panel (maybe also mention the advanced Tutorial https://www.pygmt.org/v0.13.0/tutorials/advanced/working_with_panel.html), and Bokeh
• Section "Plotting raster data": Would use the colomap "SCM/oleron" from the SC by F. Crameri instead of "geo". Maybe add a frame with default settings (frame=True) to the map.
• Section "Processing raster data": Would use the colormap "SCM/grayC" from the SC by F. Crameri instead of "gray". Maybe better to use the term "functions" instead of "modules".
• Section "Open data directly from the cloud": Why is there a .load() for zos_2015 but not for zos_2100?

     zos_2015jan = ds.zos.sel(time="2015-01-16").squeeze()
     zos_2100dec = ds.zos.sel(time="2100-12-16").squeeze()
     sealevelchange = zos_2100dec - zos_2015jan.load()

[maxrjones]

Thanks for the reviews, all! As a timing update, I will incorporate these suggestions in the morning of Dec 1 (ET). Apologies for the delay, been busy with family this week.

[maxrjones]

Thanks again for all the helpful suggestions! I believe that I made them all except standardizing the notebook metadata, because I think it'd be easier to do that as a separate PR that lints all the notebooks after the individual tutorials are merged.

[yvonnefroehlich]

Thanks again for all the helpful suggestions! I believe that I made them all [...]
Great! Just to other aspects:

• It looks like that "SPEC" is related to "scientific Python ecosystem Coordination", so the acronym for "scientific Python ecosystem" is only "spe" (https://scientific-python.org/specs/), and it makes sense to rename the file. I already fixed this for tutorial 2 in PR Tutorial 2: Fix typos #18 (sorry for introducing this typo).

• There seem to be problems with using external images within the Jupyter book on the AGU website, even the images are correctly displayed within the Juypter notebook on the related GiHub website. Please see also PR Incorrect display of images with in JN book on AGU website #17 for this issue with Tutorial 1.

[...] except standardizing the notebook metadata, because I think it'd be easier to do that as a separate PR that lints all the notebooks after the individual tutorials are merged.

Yes, agree on this point! Does this also include following a consistent coding style?

[weiji14]

Thanks @maxrjones, no worries on the delay, I didn't want to put too much pressure on you over Thanksgiving weekend, so have went ahead and renamed the notebook to tut03_spe_xarray.ipynb following @yvonnefroehlich's suggestion, and will merge this shortly in the interest of time.

[...] except standardizing the notebook metadata, because I think it'd be easier to do that as a separate PR that lints all the notebooks after the individual tutorials are merged.

Yes, agree on this point! Does this also include following a consistent coding style?

Yes, let's apply the linters from the .pre-commit-config.yaml after all the notebooks are merged.

[yvonnefroehlich]

Thanks for the reviews, all! As a timing update, I will incorporate these suggestions in the morning of Dec 1 (ET). Apologies for the delay, been busy with family this week.

Thanks @maxrjones, no worries on the delay, I didn't want to put too much pressure on you over Thanksgiving weekend [...]

I apologize, @maxrjones! Thanksgiving was totally not in my mind when we were setting up the deadlines 🙁.

[yvonnefroehlich]

Yes, let's apply the linters from the .pre-commit-config.yaml after all the notebooks are merged.

Sounds goood!