Let Session get clients by name #867
Conversation
Resolves #857
|
@jreiberkyle @cholmes review please 🙏 And just merge if you will, it's all good to go. |
jreiberkyle
left a comment
jreiberkyle
left a comment
There was a problem hiding this comment.
Some doc comments that may be helpful for @JuLeeAtPlanet
An opportunity to tweak the tests if you'd like to take it.
Yay for getting this into RC!
And test Session.client() within an async context
|
Could you please update both examples? It should only take a minute. Also, @mansi this will affect our notebooks and @emma-steuer this will affect your PR. |
And use more keyword arguments to help users understand API semantics.
| item_ids=['20200925_161029_69_2223', '20200925_161027_48_2223'], | ||
| product_bundle='analytic_udm2', | ||
| item_type='PSScene') | ||
| ], |
There was a problem hiding this comment.
Using product()'s keyword arguments makes the calls more understandable and the whole example more instructive.
There was a problem hiding this comment.
This makes it much more readable, thank you!
| # wait for completion | ||
| await client.wait(order['id'], callback=reporter.update_state) | ||
|
|
||
| # download |
There was a problem hiding this comment.
I removed obvious comments and rolled them up into a docstring.
There was a problem hiding this comment.
Let's do the same thing for the data example.
| ) | ||
|
|
||
|
|
||
| async def create_and_download(client, order_detail, directory): |
There was a problem hiding this comment.
I moved the client argument to the head of the list to make the signature of this function more like that of client.download_order(), where client (aka self) is implicitly the first argument.
| await asyncio.gather( | ||
| create_and_download(iowa_order, DOWNLOAD_DIR, client), | ||
| create_and_download(oregon_order, DOWNLOAD_DIR, client), | ||
| create_and_download(client, iowa_order, DOWNLOAD_DIR), |
There was a problem hiding this comment.
See comment above.
|
@kevinlacaille thanks for pointing out the examples! I'm going back to my ski vacation now. |
It's a proxy for a Data API search.
| import os | ||
|
|
||
| import planet | ||
|
|
There was a problem hiding this comment.
Should we get rid of the 2020 copyright on line 1?
There was a problem hiding this comment.
nope, according to @cholmes investigation, we just add a line above
|
|
||
|
|
||
| async def download_and_validate(item_id, item_type_id, asset_type_id, client): | ||
| async def download_and_validate(client, item_id, item_type_id, asset_type_id): |
There was a problem hiding this comment.
I like the positioning of client here, like self. I think you mentioned that in a comment somewhere. Great call.
kevinlacaille
left a comment
kevinlacaille
left a comment
There was a problem hiding this comment.
Looks great to me, thanks for addressing my concerns about the examples!
| } | ||
|
|
||
| oregon_images = ['20200909_182525_1014', '20200909_182524_1014'] | ||
| oregon_items = ['20200909_182525_1014', '20200909_182524_1014'] |
There was a problem hiding this comment.
Whoopsie, the images suffix was from my old days in astronomy, thanks for catching this.
This is take 2 of #858. After merging #860 (which reverted #858) the issue857 branch was in a state that I couldn't repair. So I started fresh. All the comments and examples of #858 still apply.
Resolves #857