-
Notifications
You must be signed in to change notification settings - Fork 5
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
Tutorial 4 - Geophysics (Seismology): Add JN #8
Conversation
Hi @yvonnefroehlich |
Yep, I am sorry, I mean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot, @jhtong33 , for getting started here 👍!
I made a few updates:
- I prefer to use the Scientific colour maps by F. Crameri, i.e., for topography, "oleron".
- I reduced the resolution of the image showing within the Jupyter notebook to keep the file size smaller.
- I figured out that list input is supported for the
center
andendpoint
parameters ofproject
. - Fix typo
grdproject
toproject
. - So minor code style improvements.
- I think we should be consistent and use either single (
'
) or double ("
) quotation marks; I prefer double quotation marks.
Great! Thanks for your updates. |
Hi @yvonnefroehlich |
This seem to be more complicated to implemente, please see GenericMappingTools/pygmt#3645 (comment) for details. |
Are there any major review requests on this tutorial 🙂? Otherwise, I feel we can move on and merge this tutorial in the current state and, if needed, open separat PRs for minor corrections and improvements. |
I have two observations only:
|
In this tutorial, we read the CSV file via pandas into a DataFrame. The first line is by default considered as the header line and used for the column names. Later, these column names can be used to access the single columns. pandas does not require a
Thus, I prefer to leave this file as is. |
Hm. You mean something simliar as Max does in the Xarray tutorial in the section "Processing raster data" (https://github.com/GenericMappingTools/agu24workshop/blob/add-xarray-tutorial/book/tut03_spec_xarray.ipynb) using |
My idea is just to make the maps look better. I don't think it's necessary to explain anything. So, I think you can add something like |
Ah, I first thought about |
book/tut04_geophysics.ipynb
Outdated
"3. `annotation`: Annotate contour levels.\n", | ||
"4. `limit`: Draw contours below low or above high, e.g., [-4000, 0] means drawing contour lines below sea level and above -4000 m.\n", | ||
"\n", | ||
"Before we plot the grid as contour lines, we plot the grid with color-coding using [`pygmt.Figure.grdimage`](https://www.pygmt.org/v0.13.0/api/generated/pygmt.Figure.grdimage.html). Via the `shading` parameter, we can apply an illumination to the grid or image, which can help to create visually appealing maps. By passing the argument **+a**, we use an azimuth of -45 degrees. Run the next code cell twice, first without and second with illumination, to see the difference. For more a complicated illumination, please refer to [`pygmt.grdgradient`](https://www.pygmt.org/v0.13.0/api/generated/pygmt.grdgradient.html), which was already introduced in Tutorial 3." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if and how it is possible to add links to the other Jupyter notebooks or scripts (if this is possible, maybe we can also do this in a common PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I fully understand your point. Are you suggesting that this information could be linked to the current gallery?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. What do you mean with "current gallery"?
We partly mention other tutorials, which is good. I feel it would be nice to directly insert the link. But we have the URL from GitHub (https://github.com/GenericMappingTools/agu24workshop/blob/main/book/tut0XYZ.ipynb) as well as the one from the Jupyter book (https://www.generic-mapping-tools.org/agu24workshop/XYZ.html). So depending on which website people are, the link does lead them to another "base" website. As the Jupyter book website is the "official" website for this workshop and GitHub is more for development, maybe using the URL to the Jupyter book website makes more sense here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I understand what you mean.
As the Jupyter book website is the "official" website for this workshop and GitHub is more for development, maybe using the URL to the Jupyter book website makes more sense here?
YES. Providing the URL to the Jupyter Book website will make it more convenient and readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful, thanks @yvonnefroehlich and @jhtong33 for working on this! Sorry for leaving this last, Seismology isn't my area of expertise, so I was hoping someone would jump in to review, but I trust that this is ok. Personally, I would have preferred it if the .txt and .csv file were not in the repository since they are big files, but hopefully everyone has good internet connection during the workshop 😆
I've got some minor suggestions below, which either of you can apply, and then I will try to merge this by tomorrow. After this, we can work on polishing all of the notebooks to have a consistent header, title and such.
If needed, I can adjust the period of the earthquake catalog, which might reduce the file size. (current size: 63 kb) |
Co-authored-by: Wei Ji <[email protected]>
Now worries @weiji14, @Esteban82 made already some helpful suggestions! And somethings it's actually also good to have a review from another field and also good to a have native-speaking person reading through the documentation.
Yeah! Also was thinking about what is better: people download the data during the tutorials or we provide data files within the repo, when creating the pands / geopandas tutorial. Downloading the data directly underlines that PyGMT integrates smoothly in the complete analysis workflow, and we keep the repo size small. Regarding a stable internet connection, I feel (hope) this should work (I have never been to AGU or any other big conference yet, but a working (acceptable fast) internet connection is essential for a nicely running conference) . Regarding the elevation data: The "normal" way doing this would be to simply download the built-in GMT remote dataset for the Earth relief into a Regarding the seismicity data: ObsPy is required to request and download this data. If we include this part within the tutorial, ObsPy will become a dependency, which people need to install. This was probably the reason why Jing-Hui downloaded the data externally and wrote it to a CSV file for usage in the tutorial (reading via pandas).
Sounds good! |
Yvonne mentioned a critical point: many current datasets (e.g., crustal thickness and seismic tomography) are available only as xyz columns rather than pre-processed GMT grids. If beginners fail to correctly convert xyz data into a grid, using
If including Obspy, installing and downloading IRIS dataset more relies on internet. |
Ah I see, then teaching xyz2grd would make sense here. Thanks for the extra context, this is why having people working in the same field makes sense 😉
The earthquake data in CSV format is actually not too big (64KB before, now 29KB). It's more the south_america_topography_05m.txt file that's a little large (5.0MB) 🙂, but I think it's ok to just keep it in the repository for now instead of changing the workflow. Also ok with having ObsPy included in the dependency list, but that can be done in a follow-up PR (only if there's time, otherwise the PR is ok as is). @jhtong33, I'll leave this up for a few more hours in case there's any small changes you still want to make, and will merge once you hit approve (or by the end of my work day in ~7 hours). |
Tutorial is now up at https://www.generic-mapping-tools.org/agu24workshop/tut04_geophysics.html 🎉 |
This PR adds the Jupyter notebook for Tutorial 4 - Geophysics (Seismology).
Content:
pygmt.xyz2grd
pygmt.Figure.grdcontour
pygmt.project
andpygmt.grdtrack
Feel free to fill the subsections with suggestions for code and docs🙂.
Preview:
Fixes: #6