Skip to content

#78 remove notebooks code reliant valkyrie #80

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

trey-stafford
Copy link
Contributor

Resolves #78

This removes all of the notebooks and code, except for:

  • The 01_introduction notebook, which now only contains overview information about (pre)icebridge/icesat/icesat2 altimetry data. It does not provide any code for searching/accessing/harmonizing data.
  • The corrections notebook. This remains unchanged, describing to users the importance of harmonizing the CRS of data across missions, and using a synthesized dataset as an example

@trey-stafford trey-stafford requested a review from asteiker August 5, 2024 16:45
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

github-actions bot commented Aug 5, 2024

Binder 👈 Launch a binder notebook on this branch for commit 6ec5ab2

I will automatically update this comment whenever this PR is modified

Binder 👈 Launch a binder notebook on this branch for commit 5d44168

Binder 👈 Launch a binder notebook on this branch for commit 4f3753e

Binder 👈 Launch a binder notebook on this branch for commit 50e3979

Binder 👈 Launch a binder notebook on this branch for commit f53fed5

Binder 👈 Launch a binder notebook on this branch for commit b0f2d0a

Binder 👈 Launch a binder notebook on this branch for commit e0b0db5

Binder 👈 Launch a binder notebook on this branch for commit 4ac2410

Binder 👈 Launch a binder notebook on this branch for commit 39bc777

Binder 👈 Launch a binder notebook on this branch for commit 13075d9

@trey-stafford
Copy link
Contributor Author

PR to resolve failing tests: #81

Copy link
Member

Choose a reason for hiding this comment

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

I think we'll want to have a pointer to the new iceflow library here. I can make some suggested changes directly - just wanted to add my comment in the meantime!

@asteiker asteiker requested a review from mikala-nsidc May 20, 2025 19:48
@betolink
Copy link
Member

The changes look good to me!

@mikala-nsidc
Copy link
Collaborator

In the 01-introduction notebook the link to OpenAltimetry should be updated to:

https://openaltimetry.earthdatacloud.nasa.gov/data/

Otherwise that notebook looks good to me! Still need to read through corrections.

@mikala-nsidc
Copy link
Collaborator

Nevermind about corrections notebook! I needed to read the description of the PR more carefully above.

@trey-stafford
Copy link
Contributor Author

In the 01-introduction notebook the link to OpenAltimetry should be updated to:

https://openaltimetry.earthdatacloud.nasa.gov/data/

Otherwise that notebook looks good to me! Still need to read through corrections.

Good catch! There were some other outdated references as well, so I fixed those too. e0b0db5

@@ -0,0 +1,150 @@
{
Copy link
Collaborator

@andypbarrett andypbarrett May 21, 2025

Choose a reason for hiding this comment

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

Is "Altimetry Harmonization" the best title? I think it is misleading, the page does not give any information about harmonization, just background to the missions.

Introduce both this notebook and "corrections" notebook.

In section 1. we say the notebook is interactive but there is no code to interact with, it is just a text document. Suggest combing the two notebooks.

The "pre-IceBridge" flights seem like an add-on. If we are providing a chronological introduction to NASA Cryosphere Altimetry missions, then we should start with ATM.

None of these missions measure ice thickness directly, they measure land, vegetation, ice, sea ice and ocean surface heights. Thickness and change in thickness are derived products.

Suggest introducing the missions (sections 2.1 - 2.4) before discussing the differences in file formats and reference frames.

Need information on what the file formats are and how to read them.

I would also add a table of the ITRF that correspond to the four products, and any changes that have occurred.


Reply via ReviewNB

@andypbarrett
Copy link
Collaborator

@asteiker and @trey-stafford , I wonder if the materials here could be incorporated into the iceflow notebooks. They seem to be isolated here. Alternatively, we could incorporate them into the NSIDC Data Cookbook.

A path forward might be to merge this PR, assuming we are not losing any useful information, and then open an issue in the Cookbook to incorporate the material there.

An alternative would be to incorporate the information into the UW E-Science ICESat-2 Cookbook.

That said, we could do both. There is no harm in repetition or duplication

@trey-stafford trey-stafford force-pushed the 78-remove-notebooks-code-reliant-valkyrie branch from 4ac2410 to 39bc777 Compare June 4, 2025 20:36
@trey-stafford trey-stafford force-pushed the 78-remove-notebooks-code-reliant-valkyrie branch from 39bc777 to 13075d9 Compare June 4, 2025 20:37
@trey-stafford
Copy link
Contributor Author

Following discussions between myself and @asteiker, we decided to migrate the 0_introduction.ipynb and corrections.ipynb to the iceflow repository (see nsidc/iceflow#70).

The top-level README for this repository has been updated to point users to the iceflow repository and documentation. All other iceflow-related content has been removed.

@trey-stafford trey-stafford requested a review from asteiker June 4, 2025 20:40
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this was just removed then added (no changes)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the openaltimetry links in this notebook from http://openaltimetry.org/ to https://openaltimetry.earthdatacloud.nasa.gov/data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove IceFlow notebooks and code reliant on Valkyrie
5 participants