planetlabs / notebooks

interactive notebooks from Planet Engineering
https://developers.planet.com/
Apache License 2.0
618 stars 302 forks source link

PV update LST subs examples (planet API and SH Sandbox data) #337

Closed yoann-malbeteau closed 2 months ago

yoann-malbeteau commented 3 months ago
matthew-ballard commented 2 months ago

hey, this looks good, I wasn't sure the right way to make suggestions on ipynbs, so I committed a change here, hope that works?

I only have a few questions:

matthew-ballard commented 2 months ago

added a suggested way to show how to set up auth for SH SDK here that can be referenced from all notebooks, here is the link in this branch: https://github.com/planetlabs/notebooks/tree/PV_update_LST_subs_example?tab=readme-ov-file#sentinel-hub-python-sdk

yoann-malbeteau commented 2 months ago

hey, this looks good, I wasn't sure the right way to make suggestions on ipynbs, so I committed a change here, hope that works?

It works perfectly. thank you for making the changes.

  • in numberdays_LST.pynb you have a link to EO Browser: You can also explore the area of interest in Sentinel Hub's EO Browser. This link doesn't work for me and takes me to devext.

Oh I took the same approach forest di. I will update the link and check the validity with my team.

  • Should this folder be raised out of the planet_sandbox_data folder? Personally, I think that this sandbox data folder should be removed and we should just elevate all the sandbox notebooks to workflows.

I agree with you. I think we should do it rather earlier than later because it breaks links for other documentation. Who can take this decision ? We also need to ask the forest team and move the folder. I moved it anyway

  • I think that the description of how to set up credentials is great, but I feel that each notebook isn't the right place to do this. Could we move the instructions for setting up the client_id and client_secret to the main repo README file? Then in your notebooks you can refer to that (and we can refer to it from every notebook that uses the SH APIs)

It makes sense... will do. thank you for already updating the readme.

  • I think that I would rename the folder land-surface-temperature-anomaly-analysis or something specific to the use-case/analysis that's being done. That way in the future if we added say an agriculture LST workflow, the names wouldn't conflict

oh yes... I wanted to do it and forgot. I like the names, I keep them.

matthew-ballard commented 2 months ago

I agree with you. I think we should do it rather earlier than later because it breaks links for other documentation. Who can take this decision ? We also need to ask the forest team and move the folder. I moved it anyway

ah, I didn't realize the sandbox data folder is referenced from here: https://collections.sentinel-hub.com/planet-sandbox-data/.

I realize that complicates this now, otherwise I would have said it makes to just do it.

that page I think is being managed by Markus on design or you can ask in #planet-sandbox-data on slack maybe