-
Notifications
You must be signed in to change notification settings - Fork 123
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 Landsat tutorial #692
Update Landsat tutorial #692
Conversation
cc: @robintw |
Codecov Report
@@ Coverage Diff @@
## main #692 +/- ##
=======================================
Coverage 94.30% 94.30%
=======================================
Files 77 77
Lines 11249 11249
Branches 1342 1342
=======================================
Hits 10608 10608
Misses 461 461
Partials 180 180 Continue to review full report at Codecov.
|
4b2408b
to
f13a347
Compare
f13a347
to
3142600
Compare
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.
So it's not touched by this PR right now, but there's some typing issues, e.g. in the "Set the providers" block:
Probably worth cleaning those up as well since we're hitting this file? Alternately, is there an equivalent Python typing way to do Rust's Into
/From
, since ProviderRole
is trivially convertable to/from a string?
3142600
to
7822724
Compare
@gadomski I fixed the type hinting in 7822724 and checked it using As a side note, we may want to add some |
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, LGTM. This is one of those cases where type-checking can get in the way a little bit, but I think making sure that the notebooks match the "corrent" way of using the API will save end users headaches down the road.
👍🏽 to adding nbqa
to CI.
Related Issue(s):
Description:
Updates the Landsat tutorial to be up-to-date with the current extensions and StacIO APIs.
PR Checklist:
pre-commit run --all-files
)scripts/test
)