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 SensorThings EDR Provider (USBR-115) #20

Merged
merged 15 commits into from
Sep 12, 2024
Merged

Add SensorThings EDR Provider (USBR-115) #20

merged 15 commits into from
Sep 12, 2024

Conversation

webb-ben
Copy link
Member

Description

This PR implements SensorThingsAPI as an EDR provider for pygeoapi.

Changes Made

Related Issues

geopython/pygeoapi#1807

Additional Notes

Copy link
Member

@C-Loftus C-Loftus left a comment

Choose a reason for hiding this comment

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

Looks good.

I think the only thing I would call out would be ways to make it clearer long term / other contributors. But that's up to you

pygeoapi_plugins/provider/sensorthings_edr.py Outdated Show resolved Hide resolved
pygeoapi_plugins/provider/sensorthings_edr.py Outdated Show resolved Hide resolved

@BaseEDRProvider.register()
def locations(
self, select_properties=[], bbox=[], datetime_=None, location_id=None, **kwargs
Copy link
Member

Choose a reason for hiding this comment

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

I think it it is worth type annotating these to the extent we can just since it is sometimes not super clear what datatypes pygeoapi is passing to the provider. (i.e. datetime_ could be a datetime object, string, if the client didn't know by experimenting etc.)

@webb-ben webb-ben merged commit 4968309 into master Sep 12, 2024
3 checks passed
@webb-ben webb-ben deleted the sta-edr branch September 12, 2024 22:47
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