Drop in Unidata numpy workshop notebooks #39
Conversation
|
🚀 📚 Preview for git commit SHA: 0bc6a7e at: https://607f68da19f534123ccb6bad--pythia-foundations.netlify.app |
|
These notebooks look great! I like the use of "synthetic weather data" which helps illustrate geoscience related data/plots, while also showing the user how to utilize numpy. The progression between the three notebooks is also helpful. |
| @@ -0,0 +1,840 @@ | |||
| { | |||
There was a problem hiding this comment.
This part is all new to me, I've never heard of strides.
I wonder if this part is out of scope for a foundational set of numpy lessons.
Reply via ReviewNB
There was a problem hiding this comment.
This particular use of as_strided is definitely advanced stuff and could very well be left out. The general concept of strides though of part of even Python's own slicing:
l = list(range(15)) l[::3]
gets the list with a stride of 3. Strides themselves I consider a core concept.
| @@ -0,0 +1,783 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,783 @@ | |||
| { | |||
There was a problem hiding this comment.
Add in specific geoscience example of where this would be helpful - perhaps a graphic would help with this?
Reply via ReviewNB
| @@ -0,0 +1,840 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,840 @@ | |||
| { | |||
There was a problem hiding this comment.
|
In the spirt of streamlining reviews I am removing myself as a reviewer. It looks like there are plenty of comments from others already. |
|
Hmm. I don't seem to be able to remove myself... |
Yeah, AFAIK github doesn't let you do that. Maybe @dopplershift knows differently? You can always just ignore the review request though. |
|
I plan to ignore it, and proclaim my ignorance in public :-) |
|
@dcamron just checking in. Do you think this is still doable for Wednesday? Sorry to nag :-( |
Yep! |
|
🚀 📚 Preview for git commit SHA: 2b618e9 at: https://60c7ce31d73be21c27ea36ae--pythia-foundations.netlify.app |
|
🚀 📚 Preview for git commit SHA: 2762df0 at: https://60c7dd908f2199310e4e8ae7--pythia-foundations.netlify.app |
|
Awesome, I will get to this tonight or tomorrow morning. |
| @@ -0,0 +1,674 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Good catch, I removed that.
| @@ -0,0 +1,674 @@ | |||
| { | |||
There was a problem hiding this comment.
Add a note here like "Recall that axis=-1 refers to the last axis, which in this case means the sum of x and y."
Reply via ReviewNB
| @@ -0,0 +1,674 @@ | |||
| { | |||
There was a problem hiding this comment.
This section is pretty long. Is there a way to break it up with titled subsections?
Reply via ReviewNB
| @@ -0,0 +1,674 @@ | |||
| { | |||
There was a problem hiding this comment.
This material is really good. We could introduce np.where() here to show how to implement the same logic with a numpy-native method. The point would mostly be to demonstrate that numpy has a rich library of methods for manipulating arrays that is well worth exploring. Not necessary, but that was my thought on reading this.
Reply via ReviewNB
There was a problem hiding this comment.
I think that's valuable! It also extends to potentially common uses within xarray, but maybe I'll open an issue on covering it in another notebook (separating out intermediate content?) or extending this notebook sometime in the future
| @@ -0,0 +1,975 @@ | |||
| { | |||
There was a problem hiding this comment.
This callout is rendering oddly. Can it be made consistent with the example "Warning" alert in the template?
Reply via ReviewNB
There was a problem hiding this comment.
Yep just left in something I was messing around with on accident
| @@ -0,0 +1,975 @@ | |||
| { | |||
There was a problem hiding this comment.
"Everything along the last index of the final dimension"
It may be helpful to state that *in this 3D example*, this is entirely equivalent to
c[:,:,-1]
but is useful when you don't know (or care) about the number of dimensions.
Reply via ReviewNB
| @@ -0,0 +1,975 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
These are often stylized this way in-line, check out the pandas docs and xarray docs. We can decide on a consistent stylization for non sentence starters if-needed and I don't think anyone would be too mad if I did capitalize these. Either way.
There was a problem hiding this comment.
No big deal! There are more important things to get mad about.
| @@ -0,0 +1,921 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,921 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,921 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
I left detailed comments on ReviewNB. One additional comment: it would be great to update the landing page of the numpy section to have a brief description of the topics covered in the three notebooks.
(That can be a separate PR or included here, either is fine)
|
I have to step out for a bit, but I think the notebook feedback has all been incorporated. I agree about expanding on the NumPy chapter page (and think that's valuable for all of our content so far) and will tackle that here or in a new PR later today or first thing in the morning. |
|
🚀 📚 Preview for git commit SHA: 8fe181a at: https://60c92a7372003b3ac2707e39--pythia-foundations.netlify.app |
|
Note to self (or someone else): we still need to update the Numpy landing page once this PR is merged. |
|
Looks good to me! Ready to merge! |
|
There's a little conflict with some notebook metadata. I will fix and merge. |
Ope good catch - thanks! |
|
🚀 📚 Preview for git commit SHA: 7571104 at: https://60c96b8d7c9d3c33a83bc796--pythia-foundations.netlify.app |
These are dropped in from this directory of NumPy notebooks from Unidata's workshop materials with exercises modified and branding removed. Otherwise, so far content un-modified.
Notes: