-
Notifications
You must be signed in to change notification settings - Fork 13
[Notebook] Blizzards Data Story #246
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
base: main
Are you sure you want to change the base?
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Yet To Do:
|
|
8-28 Update:
|
|
Hey @acblackford! If this is ready for review, would you mind hitting the "Ready for review" button under "Review required"? |
|
@smk0033 This is ready for review! The only thing that needs a small update is the color mapping of the snowfall accumulation data. |
| @@ -0,0 +1,6442 @@ | |||
| { | |||
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.
This seems to be in Markdown, I'd update this to a "Raw" cell and adjust the formatting to the following:
--- title: When Winter Rages description: Exploring the Power and Impact of Blizzards author: Andrew Blackford (UAH) date: August 7, 2025 execute: freeze: true ------
Also, make sure to add this notebook to the _quarto.yml file in this PR, that way it'll be visible in the docs site :)
Reply via ReviewNB
| @@ -0,0 +1,6442 @@ | |||
| { | |||
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.
Line #5. f"{RASTER_API_URL.rstrip('/')}/collections/{collection_id}"
Remove .strip('/')
Reply via ReviewNB
| @@ -0,0 +1,6442 @@ | |||
| { | |||
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'd recommend splitting this cell up, you might could add the mapping to a new cell like you did with the other examples here
Reply via ReviewNB
| @@ -0,0 +1,6442 @@ | |||
| { | |||
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.
Line #25. f"{RASTER_API_URL.rstrip('/')}/collections/{collection_id}"
Remove .rstrip('/')
Reply via ReviewNB
|
Great notebook! I've taken a first pass at a review, mostly just cleanup type things. Let me know if you have any questions, mainly about the error I'm getting with some of the utility function arguments |
|
smk0033
left a comment
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 for addressing the feedback, Andrew! A couple lingering comments, but not blockers or anything major, so I'll approve the PR:
- Under the section "Example: Total Snowfall (1950 Blizzard)", I'd recommend moving those imports to the top of the notebook with the others
- I'd recommend removing "rstrip" from a few of the lines. However, the code still works with or without it, so not too concerned here!
- Recommend splitting up a couple cells that include collection, tile request, and mapping in one. I'd just split the mapping out into it's own cell. This is noted more just for consistency, since some cells are split up and others aren't
- Unless I'm overlooking something, it seems like everything is pulled from the VEDA STAC, so I think the earthaccess library import at the top can safely be removed
Great notebook!
Description
This notebook shows datasets utilized in the blizzards data story.
What type of example is this?
Data Story
Checklist
The following checklist ensures that our notebooks are internally consistent (read more)
<collection_id>.ipynb