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 tests for point search #52

Merged
merged 2 commits into from
Nov 6, 2023
Merged

add tests for point search #52

merged 2 commits into from
Nov 6, 2023

Conversation

vincentsarago
Copy link
Member

@vincentsarago vincentsarago commented Sep 6, 2021

This PR adds tests to demonstrate a bug with the geometrysearch function when passing a geometry without area (e.g point). If exitwhenfull / skipcovered are set to true the geometrysearch function will always return []

ref: https://github.com/stac-utils/titiler-pgstac/blob/master/tests/test_titiler_pgstac.py#L46-L53

select '{"type": "FeatureCollection", "features": [{"id": "pgstac-test-item-0097"}]}'::jsonb
$$,
'Test geojsonsearch to return feature collection with all intersecting items intersecting the point'
);
Copy link
Member Author

@vincentsarago vincentsarago Sep 6, 2021

Choose a reason for hiding this comment

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

This fails right now but should pass

@bitner
Copy link
Collaborator

bitner commented Nov 6, 2023

@vincentsarago this is super old ticket, but the entire point of the geometrysearch function is to deal with areas. If we are trying to just find the intersection with a point or line, we should just be using normal search with sortby and sending an intersects geometry. I'm not sure what the case here is for anything extra that the geometrysearch is providing when it isn't an area?

@vincentsarago
Copy link
Member Author

geometrysearch function is to deal with areas

@bitner I could argue that geometry should work for any geometry 🤷‍♂️

It works great with point geometry but only if we force exitwhenfull and skipcovered to be false.

https://github.com/stac-utils/titiler-pgstac/blob/main/titiler/pgstac/mosaic.py#L183-L199

I agree that normal search would be better but again I believed that using geometrysearch should have worked with all geometry types

@vincentsarago
Copy link
Member Author

feel free to close @bitner 🙏

@bitner
Copy link
Collaborator

bitner commented Nov 6, 2023

Ah, I see the case when you are still using the query hash, but are just looking to get by point. I can make an easy fix there to the geometrysearch function.

@bitner
Copy link
Collaborator

bitner commented Nov 6, 2023

@vincentsarago I've added a check to the geometrysearch function that will set skipcovered and exitwhenfull to false whenever the input geometry is not a polygon. This passes the tests that you added (I just moved the tests to basic sql tests rather than pgtap tests to match the other existing tests).

@bitner bitner self-requested a review November 6, 2023 18:15
@bitner bitner merged commit b64bb34 into main Nov 6, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants