Skip to content

Drop in Unidata xarray workshop notebook #33

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

Merged
merged 41 commits into from
Jun 16, 2021

Conversation

dcamron
Copy link
Contributor

@dcamron dcamron commented Apr 19, 2021

This is a drop-in of this notebook from our workshop materials with interactive sections replaced with equivalent demonstrations.

Some notes:

  • Notebook definitely has crossover with data formats and map projections out of the box. The scope can be adjusted, but making relevant content that ties xarray to e.g. NetCDF will of course be important to us anyway.
  • Requires committing some example data, no remote-access content currently (also important)
  • Needs some reorganization and more jupyter-book friendly title and image formatting

@github-actions
Copy link

🚀 📚 Preview for git commit SHA: cac9b82 at: https://607dfc3beb6b1100c1da012e--pythia-foundations.netlify.app

@dcamron dcamron added the content Content related issue label Apr 20, 2021
@@ -0,0 +1,864 @@
{
Copy link
Contributor

@ktyle ktyle Apr 29, 2021

Choose a reason for hiding this comment

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

(just my initial test of ReviewNB) How about including making "Common Data Model (CDM)" a clickable link, pointing to https://www.unidata.ucar.edu/software/netcdf-java/v4.3/CDM/ ?


Reply via ReviewNB

Copy link
Member

Choose a reason for hiding this comment

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

That link appears to be dead, but I linked to https://docs.unidata.ucar.edu/netcdf-java/current/userguide/common_data_model_overview.html

@dcamron dcamron requested a review from a team April 29, 2021 17:45
@dcamron
Copy link
Contributor Author

dcamron commented Apr 29, 2021

I've pinged @ProjectPythia/education for review (I'll try to publicly self-review the content too.) As discussed in #42 and today's meeting notes, we've decided that this PR should be our main hub for focused review of a core content notebook. By our next meeting 5/13, we can plan to have this be our best current take at what we want our demonstrative notebook material to look like in terms of pacing, scope, presentation, and organization. We can then take this notebook and turn it into the template we generally plan to apply to the rest of our core teaching notebooks, as well as to provide for outside contributors to this book.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@brian-rose
Copy link
Member

Just fixed a conflict in environment.yml

@github-actions
Copy link

🚀 📚 Preview for git commit SHA: 8619fc7 at: https://608b0ed67a13565419677ecb--pythia-foundations.netlify.app

@clyne
Copy link
Contributor

clyne commented May 3, 2021

Most of my in-line comments are small nits. One more substantive comment is that reading and writing CF compliant NetCDF files is an important and complex topic. I'm wondering if a more concise and simple example should be used here to simply demonstrate read/writing data from NetCDF (without CF compliance), and having a separate book (chapter?) that gives more in depth coverage of this topic. Several examples could be included starting from the most simple, but useful CF compliant file (e.g. 2D, no projection, static) and working up to the more complex (e.g. 3D, map projection, time-varying). I think this area is one that is not well-covered elsewhere and is vitally important for folks working with CF data. What do others think?

@ktyle
Copy link
Contributor

ktyle commented May 13, 2021

With the expectation that today's meeting will focus on this excellent notebook, here is a "core dump" of my thoughts ...

  1. The overall flow and pedagogical nature of the notebook is great!
  2. I suggest splitting the notebook as it runs a bit long, particularly for one that is intended as an introduction to the library. A natural breakpoint would be when the section on the CF conventions begins.
  3. From the get-go, I would include a discussion of a Dataset in addition to the DataArray. In my mind at least, the Dataset and DataArray are the two "workhorses" of Xarray ... just as a Series and DataFrame are for Pandas.
  4. Does it make sense to include the NARR dataset for the examples (I actually do think this is a good idea, btw)? Any reason why we wouldn't defer to the air temperature dataset that is used in the Xarray package documentation examples?
  5. The initial notebook should also include at least a couple of visualizations, since, like Pandas, Xarray has a built-in interface to Matplotlib.
  6. I suspect that as we populate sections on core scientific python packages that come earlier in the sequence, such as Matplotlib and Pandas, sections that come later (such as Xarray) will need to be updated so we can add content that reiterates and reinforces material that come in the earlier sections. The CF convention material, for example, should nicely benefit from what we place in the "Data Formats" section.
  7. I am going to slightly rework some of the intro to Xarray notebooks that I have used in the two classes I teach and then submit them to the group for review. I would imagine that some of the material that I have developed will nicely complement this particular notebook.

@brian-rose
Copy link
Member

I agree with much of @ktyle's comments here. For pedagogical purposes, I support moving the CF conventions to a different notebook, and splitting the xarray content up with some more headings and shorter sections.

Two key features of xarray (to my mind) that don't appear in this notebook but could are:

  1. automatically labeled plots
  2. arithmetic between two DataArray objects to demonstrate how xarray uses coordinates to handle broadcasting.

To me point 2 is the real "killer feature" that shows the huge advantage of using xarray over plain numpy arrays when working with gridded data. I think it would be great to slip this in somewhere early on in our tutorials.

@mgrover1
Copy link
Contributor

Retweet what @ktyle and @brian-rose mentioned - I think maybe mentioning cf-xarray within the cf notebook would be good too.

Maybe a quick example of an arithmetic example, detailing how it deals with dimensionality of the datasets.

dcamron added a commit to dcamron/pythia-foundations that referenced this pull request May 27, 2021
@clyne clyne added this to the InitialPortalRelease milestone Jun 4, 2021
dcamron added a commit that referenced this pull request Jun 7, 2021
* Convert xarray (#33) to template

* Update toc with template

* Add template info to how to contribute

* Draft full template guiding text

* Use html_image myst extension.

Also fix markdown table rendering in template notebook

* Enable all MyST extensions

* Respond to reviews: tags etc

* Fix inline link to template

* Cleanup and reorder prereqs

* Fix typos

* Clarify prereqs table instructions

* Responses to review + mathjax

* Trim and clarify system requirements point

Co-authored-by: Brian Rose <[email protected]>
@jukent
Copy link
Contributor

jukent commented Jun 14, 2021

There are still lots of comments on this PR (Xarray is still XArray throughout the document, for one). I will do a thorough review after these conversations are resolved.

@github-actions
Copy link

🚀 📚 Preview for git commit SHA: 1dab418 at: https://60c7aed29b4f16222ac57797--pythia-foundations.netlify.app

@github-actions
Copy link

🚀 📚 Preview for git commit SHA: 3bb5c7b at: https://60c9911861508b72af2f6511--pythia-foundations.netlify.app

@brian-rose brian-rose closed this Jun 16, 2021
@brian-rose brian-rose reopened this Jun 16, 2021
@github-actions
Copy link

🚀 📚 Preview for git commit SHA: 7b193e8 at: https://60c996f20d3b4d620e9d871a--pythia-foundations.netlify.app

@clyne
Copy link
Contributor

clyne commented Jun 16, 2021

Wiill review asap this a.m.

@brian-rose
Copy link
Member

Assuming this gets merged in form close to what's currently here, we should open a new issue about sorting out the CF material, which is currently in a hidden notebook on this branch at core/xarray/nc-cf.ipynb

Copy link
Contributor

@jukent jukent left a comment

Choose a reason for hiding this comment

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

This is a big improvement! Just a few small changes.

@clyne clyne self-requested a review June 16, 2021 15:17
Copy link
Contributor

@clyne clyne left a comment

Choose a reason for hiding this comment

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

Much improved, Brian! I just had a few nits, then should be good to go.

@brian-rose
Copy link
Member

Thanks for the reviews @jukent and @clyne. I'll get the suggested changes pushed up shortly.

@brian-rose
Copy link
Member

@clyne @jukent as soon as this builds, it's ready for a final review and approval.

@github-actions
Copy link

🚀 📚 Preview for git commit SHA: 5541912 at: https://60ca28a04a3bb80166f5d430--pythia-foundations.netlify.app

@brian-rose brian-rose requested a review from jukent June 16, 2021 16:50
@brian-rose brian-rose requested a review from clyne June 16, 2021 16:51
@clyne
Copy link
Contributor

clyne commented Jun 16, 2021

In meetings until 11:30am MT, but will jump right on it then

Copy link
Contributor

@clyne clyne left a comment

Choose a reason for hiding this comment

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

Awesome! Approved and merging.

@clyne clyne merged commit ba4e290 into ProjectPythia:main Jun 16, 2021
dcamron added a commit to dcamron/pythia-foundations that referenced this pull request Jun 16, 2021
dcamron added a commit that referenced this pull request Jun 16, 2021
* Cleanup initial release draft

Clarify organization, trim file names, verify template-based
consistency. Clear out extra metadata and run all notebooks on standard
kernel.

* Incorporate #33 into cleanup

* Incorporate feedback
@dcamron dcamron deleted the core-xarray branch June 16, 2021 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Content related issue ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants