Skip to content

geometry relation support for data search #1205

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.

Sign up for GitHub

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

Open
ischneider wants to merge 1 commit into
base: main
Choose a base branch
from
Open

geometry relation support for data search #1205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 11 additions & 8 deletions planet/cli/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ def data(ctx, base_url):
ctx.obj['BASE_URL'] = base_url


# TODO: filter().
def geom_to_filter(ctx, param, value: Optional[dict]) -> Optional[dict]:
return data_filter.geometry_filter(value) if value else None


def assets_to_filter(ctx, param, assets: List[str]) -> Optional[dict]:
# TODO: validate and normalize
return data_filter.asset_filter(assets) if assets else None
Expand Down Expand Up @@ -172,9 +167,14 @@ def _func(obj):
DATETIME can be an RFC3339 or ISO 8601 string.""")
@click.option('--geom',
type=types.JSON(),
callback=geom_to_filter,
help="""Filter to items that overlap a given geometry. Can be a
help="""Filter to items with a given geometry. Can be a
json string, filename, or '-' for stdin.""")
@click.option(
"--geom-relation",
type=click.Choice(["intersects", "contains", "within", "disjoint"]),
help="""Optional geometry search refinement, defaults to intersects.
May also be contains, within, or disjoint.""",
)
Comment on lines +175 to +177
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The help text contains redundant information. It states "defaults to intersects" and then repeats "May also be contains, within, or disjoint" which doesn't include intersects in the list. The help text should clarify that intersects is one of the four options and is the default, for example: "Geometry search relation. Options are intersects (default), contains, within, or disjoint."

Suggested change
help="""Optional geometry search refinement, defaults to intersects.
May also be contains, within, or disjoint.""",
)
help="""Geometry search relation. Options are intersects (default), contains, within, or disjoint.""",
)

Copilot uses AI. Check for mistakes.
@click.option('--number-in',
type=click.Tuple([types.Field(), types.CommaSeparatedFloat()]),
multiple=True,
Expand Down Expand Up @@ -224,6 +224,7 @@ def filter(ctx,
asset,
date_range,
geom,
geom_relation,
number_in,
nrange,
string_in,
Expand All @@ -245,9 +246,11 @@ def filter(ctx,
permission = data_filter.permission_filter() if permission else None
std_quality = data_filter.std_quality_filter() if std_quality else None

geometry = data_filter.geometry_filter(geom, geom_relation)
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The geometry_filter function is called unconditionally, but it will fail when geom is None (which happens when --geom is not specified). The removed geom_to_filter callback previously handled this by only calling geometry_filter when geom was not None. This should be changed to: geometry = data_filter.geometry_filter(geom, geom_relation) if geom else None

Suggested change
geometry = data_filter.geometry_filter(geom, geom_relation)
geometry = data_filter.geometry_filter(geom, geom_relation) if geom else None

Copilot uses AI. Check for mistakes.

filter_options = (asset,
date_range,
geom,
geometry,
number_in,
nrange,
string_in,
Expand Down
9 changes: 8 additions & 1 deletion planet/data_filter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def update_filter(field_name: str,
callback=_datetime_to_rfc3339)


def geometry_filter(geom: dict) -> dict:
def geometry_filter(geom: dict, relation: str = None) -> dict:
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The type annotation should indicate that relation is Optional. Change relation: str = None to relation: Optional[str] = None to properly express that this parameter can be None.

Suggested change
def geometry_filter(geom: dict, relation: str = None) -> dict:
def geometry_filter(geom: dict, relation: Optional[str] = None) -> dict:

Copilot uses AI. Check for mistakes.
"""Create a GeometryFilter

The GeometryFilter can be used to search for items with a footprint
Expand All @@ -237,10 +237,17 @@ def geometry_filter(geom: dict) -> dict:
Parameters:
geom: GeoJSON describing the filter geometry, feature, or feature
collection.
relation: Optional geometry search refinement, defaults to intersects.
May also be contains, within, or disjoint.
"""
geom_filter = _field_filter('GeometryFilter',
field_name='geometry',
config=geojson.validate_geom_as_geojson(geom))
if relation:
allowed = {"intersects", "contains", "disjoint", "within"}
if relation not in allowed:
raise ValueError(f"relation must be one of {allowed}")
geom_filter["relation"] = relation
return geom_filter


Expand Down
21 changes: 10 additions & 11 deletions tests/integration/test_data_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ def _func(filter1, filter2):
return _func


@respx.mock
def test_data_filter_defaults(invoke, assert_and_filters_equal):

result = invoke(["filter"])
Expand All @@ -183,7 +182,6 @@ def test_data_filter_defaults(invoke, assert_and_filters_equal):
assert_and_filters_equal(json.loads(result.output), empty_filter)


@respx.mock
def test_data_filter_permission(invoke, assert_and_filters_equal):
result = invoke(["filter", "--permission"])
assert result.exit_code == 0
Expand All @@ -197,7 +195,6 @@ def test_data_filter_permission(invoke, assert_and_filters_equal):
assert_and_filters_equal(json.loads(result.output), expected_filt)


@respx.mock
def test_data_filter_std_quality(invoke, assert_and_filters_equal):
result = invoke(["filter", '--std-quality'])
assert result.exit_code == 0
Expand Down Expand Up @@ -230,7 +227,6 @@ def test_data_filter_asset(asset, expected, invoke, assert_and_filters_equal):
assert_and_filters_equal(json.loads(result.output), expected_filt)


@respx.mock
def test_data_filter_date_range_success(invoke, assert_and_filters_equal):
"""Check filter is created correctly and that multiple options results in
multiple filters"""
Expand Down Expand Up @@ -262,13 +258,11 @@ def test_data_filter_date_range_success(invoke, assert_and_filters_equal):
assert_and_filters_equal(json.loads(result.output), expected_filt)


@respx.mock
def test_data_filter_date_range_invalid(invoke):
result = invoke(["filter"] + '--date-range field gt 2021'.split())
assert result.exit_code == 2


@respx.mock
@pytest.mark.parametrize("geom_fixture",
[('geom_geojson'), ('feature_geojson'),
('featurecollection_geojson')])
Expand Down Expand Up @@ -296,7 +290,16 @@ def test_data_filter_geom(geom_fixture,
assert_and_filters_equal(json.loads(result.output), expected_filt)


@respx.mock
def test_data_filter_geom_relation(request, invoke):
geom = request.getfixturevalue("geom_geojson")
geom_str = json.dumps(geom)
result = invoke(["filter", f'--geom={geom_str}', '--geom-relation=disjoint'])
assert result.exit_code == 0

and_filter = json.loads(result.output)
assert and_filter["config"][0]["relation"] == "disjoint"

Comment on lines +293 to +301
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

Consider adding a test case to verify the behavior when --geom-relation is specified without --geom. This should ideally be handled gracefully (either ignored or produce a clear error message).

Copilot uses AI. Check for mistakes.

def test_data_filter_number_in_success(invoke, assert_and_filters_equal):

result = invoke(["filter"] + '--number-in field 1'.split() +
Expand All @@ -317,14 +320,12 @@ def test_data_filter_number_in_success(invoke, assert_and_filters_equal):
assert_and_filters_equal(json.loads(result.output), expected_filt)


@respx.mock
def test_data_filter_number_in_badparam(invoke, assert_and_filters_equal):

result = invoke(["filter"] + '--number-in field 1,str'.split())
assert result.exit_code == 2


@respx.mock
def test_data_filter_range(invoke, assert_and_filters_equal):
"""Check filter is created correctly, that multiple options results in
multiple filters, and that floats are processed correctly."""
Expand Down Expand Up @@ -352,7 +353,6 @@ def test_data_filter_range(invoke, assert_and_filters_equal):
assert_and_filters_equal(json.loads(result.output), expected_filt)


@respx.mock
def test_data_filter_string_in(invoke, assert_and_filters_equal):

result = invoke(["filter"] + '--string-in field foo'.split() +
Expand All @@ -375,7 +375,6 @@ def test_data_filter_string_in(invoke, assert_and_filters_equal):
assert_and_filters_equal(json.loads(result.output), expected_filt)


@respx.mock
def test_data_filter_update(invoke, assert_and_filters_equal):
"""Check filter is created correctly and that multiple options results in
multiple filters"""
Expand Down
Loading