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

update E5 dataset access #11

Merged
merged 25 commits into from
May 24, 2022
Merged

update E5 dataset access #11

merged 25 commits into from
May 24, 2022

Conversation

fnattino
Copy link
Collaborator

@fnattino fnattino commented Mar 16, 2022

Closes #2

@fnattino fnattino marked this pull request as ready for review March 16, 2022 16:24
@fnattino
Copy link
Collaborator Author

Hey @SarahAlidoost, since you already had a look at this episode, would you mind checking how do you see it now as first after the background episodes (new episode 5)?

@SarahAlidoost SarahAlidoost self-requested a review March 17, 2022 09:00
Copy link
Member

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

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

@fnattino nice addition, well done 👍 I left some minor comments on the changed files, I also committed some formatting and editorial fixes, as we discussed.

I noticed that the lesson does not include a solution for the first exercise. Can we provide solutions, for example some screenshots of the results?
Also, can we replace the link to the github spec specification with the link to its readme i.e. https://github.com/radiantearth/stac-spec#readme ?

@fnattino
Copy link
Collaborator Author

Thanks @SarahAlidoost - and good point about the solution to the first episode. I'll add some details there!

@fnattino
Copy link
Collaborator Author

Solution with screenshots added - how do you see it @SarahAlidoost ?

@fnattino fnattino requested a review from SarahAlidoost March 18, 2022 15:55
Copy link
Member

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

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

@fnattino thanks for addressing my comments. very nice job 👍 As we discussed, we can still add something like "this is an example and based on what item you selected, the image in the preview can be different" to the solution. I approved this PR.

@rbavery rbavery changed the title update E5 update E5 dataset access Mar 18, 2022
fnattino added 7 commits May 13, 2022 16:17
The newest version enforces requirements not compatible with the STAC API 0.9 (used in the Earth Search portal)
Landsat assets provided by the Earth Search STAC catalog are no longer available
# Conflicts:
#	files/environment.yaml
#	setup.md
@fnattino
Copy link
Collaborator Author

Hi @rbavery , this episode is also ready if/when you have time to review it!

Since this episode will become the first of the lesson, I have simplified it a bit, removing things that will be introduced later like cropping a raster.

The most important change is in the last exercise, which used to make use of Landsat data from the Earth Search STAC endpoint. I have noticed that the datasets are no longer available, so I switched STAC endpoint to the NASA CMR one and modified the exercise to ask for Landsat metadata from the HLS collection. I think introducing this second STAC endpoint might actually be a good thing, since this will be used also in the stackstac/odc-stac episode and it will illustrate how public catalogs can also include protected data.

Copy link
Collaborator

@rbavery rbavery left a comment

Choose a reason for hiding this comment

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

Hey @fnattino this looks awesome. Can you update the url to the episode and the table of contents so that this is now lesson 5? I'll give this a more thorough run through tomorrow.

_episodes/19-access-data.md Show resolved Hide resolved
@fnattino
Copy link
Collaborator Author

Thanks a lot @rbavery! I have waited to change the episode file name to facilitate the review (I am afraid that detailed changes will not be highlighted here on GH if I rename the file)- will take care of that just before merging!

Copy link
Collaborator

@rbavery rbavery left a comment

Choose a reason for hiding this comment

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

Great stuff @fnattino . The only thing I'm noticing is that the notebook errors and is using a different collection from the markdown lesson: collection = "landsat-8-l1-c1". can this nb be updated to reflect the markdown? or feel free to delete the nb. I don't mind having the notebooks tracked in vc for convenience, but also it may be easier to maintain just one source of truth for the lessons.

@fnattino
Copy link
Collaborator Author

Great @rbavery, and thanks for pointing out the issue of the notebook. I will remove it for now, if we see the need for it we could then add at once notebooks for all episodes at once.

@fnattino fnattino merged commit c91e4c5 into gh-pages May 24, 2022
@fnattino fnattino deleted the update-episode-5 branch May 24, 2022 09:25
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.

Episode 5: Raster data access/retrieval episode
4 participants