Skip to content
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

Review/cog optimized geotiffs #98

Merged
merged 4 commits into from
Jan 3, 2024
Merged

Conversation

zacdezgeo
Copy link
Collaborator

What I Changed

I mainly modified some of the text of both COG notebooks. The only modification to code blocks was removing comments that explain the code to a markdown cell preceding the code block. It's a personal preference that we may want to revisit.

How to Test It

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -9,6 +9,7 @@
]
Copy link
Contributor

@wildintellect wildintellect Dec 21, 2023

Choose a reason for hiding this comment

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

TODO: for later, we should add a section showing a verification that the values of the data have not changed in the conversion. This question/issue actually came up recently.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Opened issue #99

@@ -9,6 +9,7 @@
]
Copy link
Contributor

@wildintellect wildintellect Dec 21, 2023

Choose a reason for hiding this comment

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

we should cross link this section the the new noteboook.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed!

@@ -1,19 +1,20 @@
{
Copy link
Contributor

@wildintellect wildintellect Dec 21, 2023

Choose a reason for hiding this comment

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

No it's Types of Data - Nominal, Ordinal, Discrete, Continuous. Also the variance plays a role as well has how much NA data there is.

Data Type to me implies, byte, integer, float, etc...


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed it back and added types of data in parenthesis for clarity of context.

@@ -1,19 +1,20 @@
{
Copy link
Contributor

@wildintellect wildintellect Dec 21, 2023

Choose a reason for hiding this comment

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

removing over changes the meaning, perhaps over-represents is the correct term


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Contributor

@wildintellect wildintellect left a comment

Choose a reason for hiding this comment

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

I just realized neither notebook mentions how to get an EarthData Login https://urs.earthdata.nasa.gov/home.

@zacharyDez either you can add, and I'll re-approve. Or you can merge and I'll add to my PR #95

@zacdezgeo
Copy link
Collaborator Author

@wildintellect ; I added how to get an Earth Data login. Seems like approval is not required to merge to your PR. Let me know if there's anything I can help with.

@zacdezgeo zacdezgeo merged commit 93d7fae into staging Jan 3, 2024
2 checks passed
@zacdezgeo zacdezgeo deleted the review/cog-optimized-geotiffs branch January 3, 2024 14:10
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.

2 participants