Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
2.1.0 (TBD)

Added:
- The subscription_request.build_request function has a new option to clip to
the subscription's source geometry. This is a preview of the default
behavior of the next version of the Subscriptions API.

2.0.3 (2023-06-28)

Changed:
Expand Down
16 changes: 8 additions & 8 deletions planet/cli/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ async def list_subscription_results_cmd(ctx,
'--notifications',
type=types.JSON(),
help='Notifications JSON. Can be a string, filename, or - for stdin.')
@click.option('--tools',
type=types.JSON(),
help='Toolchain JSON. Can be a string, filename, or - for stdin.'
)
@click.option(
'--tools',
type=types.JSON(),
help='Toolchain JSON. Can be a string, filename, or - for stdin.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is all on yapf and not explained by anything in its change log 😒

Copy link
Contributor Author

@sgillies sgillies Jul 3, 2023

Choose a reason for hiding this comment

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

And it's version dependent. With Python 3.7, yapf 0.40.1 puts '--tools' on line 208. WTH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're now using Python 3.8 for the CI linting checks.

@pretty
def request(name, source, delivery, notifications, tools, pretty):
"""Generate a subscriptions request."""
Expand Down Expand Up @@ -247,10 +247,10 @@ def request(name, source, delivery, notifications, tools, pretty):
@click.option('--rrule',
type=str,
help='iCalendar recurrance rule to specify recurrances.')
@click.option('--filter',
type=types.JSON(),
help='Search filter. Can be a string, filename, or - for stdin.'
)
@click.option(
'--filter',
type=types.JSON(),
help='Search filter. Can be a string, filename, or - for stdin.')
@pretty
def request_catalog(item_types,
asset_types,
Expand Down
64 changes: 42 additions & 22 deletions planet/subscription_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# the License.
"""Functionality for preparing subscription requests."""
from datetime import datetime
from typing import Any, Dict, Optional, List
from typing import Any, Dict, Optional, List, Mapping

from . import geojson, specs
from .exceptions import ClientError
Expand Down Expand Up @@ -45,12 +45,27 @@


def build_request(name: str,
source: dict,
delivery: dict,
notifications: Optional[dict] = None,
tools: Optional[List[dict]] = None) -> dict:
source: Mapping,
Copy link
Contributor Author

@sgillies sgillies Jun 30, 2023

Choose a reason for hiding this comment

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

Typing best practices (https://typing.readthedocs.io/en/latest/source/best_practices.html) is to accept mappings (most general) and return dicts (most specific). This follows Postel's Law.

delivery: Mapping,
notifications: Optional[Mapping] = None,
tools: Optional[List[Mapping]] = None,
clip_to_source=False) -> dict:
"""Prepare a subscriptions request.

Parameters:
name: Name of the subscription.
source: A source for the subscription, i.e. catalog.
delivery: A delivery mechanism e.g. GCS, AWS, Azure, or OCS.
notifications: Specify notifications via email/webhook.
tools: Tools to apply to the products. The order of operation
is determined by the service.
clip_to_source: whether to clip to the source geometry or not
(the default). If True a clip configuration will be added
to the list of requested tools unless an existing clip tool
exists. NOTE: the next version of the Subscription API will
remove the clip tool option and always clip to the source
geometry. Thus this is a preview of the next API version's
default behavior.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving this above the example.


```python
>>> from datetime import datetime
Expand All @@ -77,31 +92,36 @@ def build_request(name: str,

```

Parameters:
name: Name of the subscription.
source: A source for the subscription, i.e. catalog.
delivery: A delivery mechanism e.g. GCS, AWS, Azure, or OCS.
notifications: Specify notifications via email/webhook.
tools: Tools to apply to the products. Order defines
the toolchain order of operatations.
"""
details = {"name": name, "source": source, "delivery": delivery}
details = {
"name": name, "source": dict(source), "delivery": dict(delivery)
Copy link
Contributor Author

@sgillies sgillies Jun 30, 2023

Choose a reason for hiding this comment

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

Here is the consequence of accepting mappings: we need to explicitly convert them to dicts here. The upside is that we will never modify dicts that users pass in to this function, which can cause subtle bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We copy information now, which has some overhead, but makes calling code more reliable. And building a subscriptions request doesn't need to be super optimized.

}

if notifications:
details['notifications'] = notifications
details['notifications'] = dict(notifications)

if tools:
details['tools'] = tools
tool_list = [dict(tool) for tool in tools]
if clip_to_source and not any(
tool.get('type', None) == 'clip' for tool in tool_list):
tool_list.append({
'type': 'clip',
'parameters': {
'aoi': source['parameters']['geometry']
}
})

details['tools'] = tool_list

return details


def catalog_source(
item_types: List[str],
asset_types: List[str],
geometry: dict,
geometry: Mapping,
start_time: datetime,
filter: Optional[dict] = None,
filter: Optional[Mapping] = None,
end_time: Optional[datetime] = None,
rrule: Optional[str] = None,
) -> dict:
Expand Down Expand Up @@ -142,7 +162,7 @@ def catalog_source(
parameters = {
"item_types": item_types,
"asset_types": asset_types,
"geometry": geojson.as_geom(geometry),
"geometry": geojson.as_geom(dict(geometry)),
}

try:
Expand All @@ -151,7 +171,7 @@ def catalog_source(
raise ClientError('Could not convert start_time to an iso string')

if filter:
parameters['filter'] = filter
parameters['filter'] = dict(filter)

if end_time:
try:
Expand Down Expand Up @@ -348,7 +368,7 @@ def band_math_tool(b1: str,
return _tool('bandmath', parameters)


def clip_tool(aoi: dict) -> dict:
def clip_tool(aoi: Mapping) -> dict:
'''Specify a subscriptions API clip tool.

Imagery and udm files will be clipped to your area of interest. nodata
Expand All @@ -370,12 +390,12 @@ def clip_tool(aoi: dict) -> dict:
'''
valid_types = ['Polygon', 'MultiPolygon']

geom = geojson.as_geom(aoi)
geom = geojson.as_geom(dict(aoi))
if geom['type'].lower() not in [v.lower() for v in valid_types]:
raise ClientError(
f'Invalid geometry type: {geom["type"]} is not in {valid_types}.')

return _tool('clip', {'aoi': aoi})
return _tool('clip', {'aoi': geom})


def file_format_tool(file_format: str) -> dict:
Expand Down
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@ fail_under = 98
[yapf]
based_on_style = pep8
split_all_top_level_comma_separated_values=true

[flake8]
ignore = E126,E501,W50
32 changes: 16 additions & 16 deletions tests/integration/test_data_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,17 +558,17 @@ async def test_run_search_doesnotexist(session):
async def test_get_stats_success(search_filter, session):

page_response = {
"buckets": [{
"count": 433638, "start_time": "2022-01-01T00:00:00.000000Z"
},
{
"count": 431924,
"start_time": "2022-01-02T00:00:00.000000Z"
},
{
"count": 417138,
"start_time": "2022-01-03T00:00:00.000000Z"
}]
"buckets": [
{
"count": 433638, "start_time": "2022-01-01T00:00:00.000000Z"
},
{
"count": 431924, "start_time": "2022-01-02T00:00:00.000000Z"
},
{
"count": 417138, "start_time": "2022-01-03T00:00:00.000000Z"
},
],
}
mock_resp = httpx.Response(HTTPStatus.OK, json=page_response)
respx.post(TEST_STATS_URL).return_value = mock_resp
Expand Down Expand Up @@ -875,11 +875,11 @@ async def _stream_img():

@respx.mock
@pytest.mark.anyio
@pytest.mark.parametrize("hashes_match, md5_entry, expectation",
[(True, True, does_not_raise()),
(False, True, pytest.raises(exceptions.ClientError)),
(True, False, pytest.raises(exceptions.ClientError))]
)
@pytest.mark.parametrize(
"hashes_match, md5_entry, expectation",
[(True, True, does_not_raise()),
(False, True, pytest.raises(exceptions.ClientError)),
(True, False, pytest.raises(exceptions.ClientError))])
async def test_validate_checksum(hashes_match, md5_entry, expectation, tmpdir):
test_bytes = b'foo bar'
testfile = Path(tmpdir / 'test.txt')
Expand Down
26 changes: 25 additions & 1 deletion tests/unit/test_subscription_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,31 @@ def test_build_request_success(geom_geojson):
assert res == expected


def test_build_request_clip_to_source(geom_geojson):
source = {
"type": "catalog",
"parameters": {
"geometry": geom_geojson,
"start_time": "2021-03-01T00:00:00Z",
"end_time": "2023-11-01T00:00:00Z",
"rrule": "FREQ=MONTHLY;BYMONTH=3,4,5,6,7,8,9,10",
"item_types": ["PSScene"],
"asset_types": ["ortho_analytic_4b"]
}
}
res = subscription_request.build_request(
'test',
source=source,
delivery={},
tools=[{
'type': 'hammer'
}],
clip_to_source=True,
)
assert res["tools"][1]["type"] == "clip"
assert res["tools"][1]["parameters"]["aoi"] == geom_geojson


def test_catalog_source_success(geom_geojson):
res = subscription_request.catalog_source(
item_types=["PSScene"],
Expand Down Expand Up @@ -230,7 +255,6 @@ def test_band_math_tool_invalid_pixel_type():

def test_clip_tool_success(geom_geojson):
res = subscription_request.clip_tool(geom_geojson)

expected = {"type": "clip", "parameters": {"aoi": geom_geojson}}
assert res == expected

Expand Down