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

Add a planetary_variable_source subscription helper #976

Merged
merged 3 commits into from
Jul 11, 2023
Merged

Conversation

sgillies
Copy link
Contributor

@sgillies sgillies commented Jul 7, 2023

Resolves #966. API doc improvements for catalog_source() are included in this PR.

@sgillies sgillies added this to the 2.1.0 milestone Jul 7, 2023
@sgillies sgillies self-assigned this Jul 7, 2023
@sgillies
Copy link
Contributor Author

sgillies commented Jul 7, 2023

@cpaulik @guyon-planet while I work on #933, would you be willing to see if this is what you're looking for in a PV source helper?

Copy link
Contributor

@kevinlacaille kevinlacaille left a comment

Choose a reason for hiding this comment

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

A few small suggestions and 1 must-fix typo in the example code. Otherwise, looks good to me!

CHANGES.txt Show resolved Hide resolved
planet/subscription_request.py Show resolved Hide resolved
planet/subscription_request.py Show resolved Hide resolved
planet/subscription_request.py Outdated Show resolved Hide resolved
tests/unit/test_subscription_request.py Show resolved Hide resolved
@@ -212,6 +219,86 @@ def catalog_source(
return {"type": "catalog", "parameters": parameters}


def planetary_variable_source(
var_type: Literal["biomass_proxy",
Copy link

Choose a reason for hiding this comment

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

Just wanted to point out that internally we are already using more sources for upcoming forest products.

I'm not sure if there is a way to not duplicate these lists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cpaulik I'm going off what we have at https://developers.planet.com/docs/subscriptions/pvs-subs/#planetary-variables-types-and-ids. Do you know if we publish a machine-readable version of this table for customers?

I'm using the Literal type hints here so that developers get value hints from their IDEs. The SDK does not at this time validate the values at runtime. Thus you can use any values you want, including unavailable PV types and ids. See for example https://github.com/planetlabs/planet-client-python/pull/982/files#diff-661d60b3722e519e66594c89a655b8aa191b919ac27c98ecffde58f984ab120eR355.

Copy link

Choose a reason for hiding this comment

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

You sparked my curiosity. Since our API docs spell them out I did some digging and found that https://api.planet.com/subscriptions/v1/spec includes the sources types under schemas.PVSourceTypes

@guyon-planet
Copy link
Contributor

Quickly tested it, looks great, thanks!

@guyon-planet guyon-planet merged commit 3942a0b into main Jul 11, 2023
9 checks passed
@sgillies sgillies mentioned this pull request Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a Planetary Variable source helper to the subscription_request module
4 participants