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 bare bone APIs for observation and event #186

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shifucun
Copy link
Contributor

@shifucun shifucun commented Nov 7, 2022

This PR aims to finalize the naming of Python API for observation and event.



def event_series_within_place(event_type: str, containing_place: str,
date: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to use variable here too? Couple of options:

  1. Use the same API as observation_series() and friends, except, depending on the variable (Count_EarthquakeEvent vs. List_EarthquakeEvent), the returned value will have entities per date, rather than a value per date.

  2. Use very similar API structure as observation_series(), but these are "event" versions. We use the existing variables (e.g., Count_EarthquakeEvent), and this new API returns entities per data point.

Copy link
Contributor

Choose a reason for hiding this comment

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

my preference would be for (2)
rather than event, should we use entity to be more extensible?


from typing import List


Copy link
Contributor

Choose a reason for hiding this comment

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

document return value?

@pradh
Copy link
Contributor

pradh commented Nov 7, 2022

Do we want this as py-only API or REST API first?

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.

3 participants