-
Notifications
You must be signed in to change notification settings - Fork 311
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
Google Earth Engine integration notebook #220
Conversation
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.
Looks good, I was able to run this locally. Just some small comments
"source": [ | ||
"# Google Earth Engine configuration\n", | ||
"cloud_config = planet.order_request.google_earth_engine(\n", | ||
" project='planet-devrel-dev', collection='gee-integration-testing')\n", |
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.
#nit might be nice to have the GEE delivery options as consts at the top of the notebook
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.
If this were a regular script I would 100% do this, but I fear that would be a little inconsistent with how we typically lay out our notebooks. Thoughts on this @mkshah605?
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 be open to it! It might make sense so users can easily change their credentials. It's similar to how I laid out this Subscriptions notebook, where people can define their bucket and gcs key in cell #2. Definitely open to your thoughts!
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've thought about it and we only use the project and collection names a single time, just in this cell, and it feels a bit unnecessary to define these as constants. @mkshah605, it makes sense in your subscriptions notebook, as you use bucket
and gcs_key
more than once in your notebook, but in this case I seem to justify creating two more variables. Are you opposed to leaving them as is?
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.
yes that makes sense. I am good with leaving them in here
This notebook is well set up to interact directly with the Python SDK as expected, this is not backward compatible, and maybe we need to include the SDK version information needed somewhere on the notebook. Change cell 1 to this
Images might be delivered while the script polls for status to change to success, but images might be delivered to Earth Engine collection during the wait time, so I would include that information |
Note that only clipping and harmonization are the only two tools that are compatible with GEE in cell 15. |
This notebook outlines how to integrate Planet with Google Earth Engine (GEE) using Planet's service account and the Orders API. There will be a follow up notebook, which will outline how to integrate with GEE using a custom service account.
Closes #210