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

[covidcast] Add support for querying multiple signals #116

Closed
krivard opened this issue Jun 29, 2020 · 6 comments · Fixed by cmu-delphi/delphi-epidata#170
Closed

[covidcast] Add support for querying multiple signals #116

krivard opened this issue Jun 29, 2020 · 6 comments · Fixed by cmu-delphi/delphi-epidata#170
Assignees
Milestone

Comments

@krivard
Copy link
Contributor

krivard commented Jun 29, 2020

Needed for cmu-delphi/www-covidcast#178: Expanded tooltips. Grab all several signals available for a region / set of regions, the same way we can grab all regions.

@krivard krivard transferred this issue from cmu-delphi/delphi-epidata Jun 29, 2020
@capnrefsmmat
Copy link
Contributor

We may actually prefer to support a fixed list in the GET query, and not *, just because there are so many indicators and variants of indicators in the API. Users who really want everything can use the metadata endpoint to find them all.

@krivard krivard changed the title [covidcast] Add support for * in signal field [covidcast] Add support for querying multiple signals Jun 30, 2020
@krivard
Copy link
Contributor Author

krivard commented Jun 30, 2020

Hey @adamperer @rhansolo, how would you like this data to come back?

Example:

source signal time type time value geo type geo value value stderr sample size
magic-words abracadabra day 20200414 state az 123 12 456
magic-words abracadabra day 20200415 state az 111 11 444
magic-words xyzzy day 20200414 state az 234 23 345
magic-words xyzzy day 20200415 state az 222 22 333

Query: Epidata.covidcast('magic-words', ['abracadabra','xyzzy'], 'day', 'state', [20200414, 20200415], 'az')

Response option 1: add key signal to each element of epidata list

{
  'result': 1,
  'message': 'success',
  'epidata': [{
      'signal': 'abracadabra',
      'time_value': 20200414,
      'geo_value': 'az',
      'value': 123,
      'stderr': 12,
      'sample_size': 456,
      'direction': 0,
    },{
      'signal': 'abracadabra',
      'time_value': 20200415,
      'geo_value': 'az',
      'value': 111,
      'stderr': 11,
      'sample_size': 444,
      'direction': 0,
    },{
      'signal': 'xyzzy',
      'time_value': 20200414,
      'geo_value': 'az',
      'value': 234,
      'stderr': 23,
      'sample_size': 345,
      'direction': 0,
    },{
      'signal': 'xyzzy',
      'time_value': 20200415,
      'geo_value': 'az',
      'value': 222,
      'stderr': 22,
      'sample_size': 333,
      'direction': 0,
    }],
}

Response option 2: Split out epidata list by signal:

{
  'result': 1,
  'message': 'success',
  'epidata': {
    'abracadabra': [{
        'time_value': 20200414,
        'geo_value': 'az',
        'value': 123,
        'stderr': 12,
        'sample_size': 456,
        'direction': 0,
      }, {
        'time_value': 20200415,
        'geo_value': 'az',
        'value': 111,
        'stderr': 11,
        'sample_size': 444,
        'direction': 0,
      }],
    'xyzzy': [{
        'time_value': 20200414,
        'geo_value': 'az',
        'value': 234,
        'stderr': 23,
        'sample_size': 345,
        'direction': 0,
      },{
        'time_value': 20200415,
        'geo_value': 'az',
        'value': 222,
        'stderr': 22,
        'sample_size': 333,
        'direction': 0,
      }]
  }
}

@krivard krivard self-assigned this Jun 30, 2020
@capnrefsmmat
Copy link
Contributor

Won't we also need to support selecting signals from multiple sources? That would involve specifying (source, signal) pairs, and a source column in the response.

@krivard
Copy link
Contributor Author

krivard commented Jun 30, 2020

That would be a much more substantial change in the layout of both the API query and the SQL query. IIRC the immediate use case for this is the cases & deaths views, followed by the single-location view we have planned for future development.

For cases and deaths, the map needs to pull (smoothed, raw) X (num, prop) X (incidence, cumulative) to complete ✨
the tooltip of Roni's dreams ✨. With the current API that's 8 queries, but they're all from the same source. That makes it relatively simple to replace the signal parameter with a signals parameter that works the same way that time_values does. We get backwards-compatibility for free, and no one's clients would break.

For the single-location view, I'm assuming we want to display all and only the other signals we currently display in the map: doctor-visits x1 + fb-survey x2 + safegraph x2 + ght x1 + combined x1 + jhu-csse/usa-facts x8. With the current API that's 15 queries. With the above support for multiple signals on a single source that's 6 queries. To reduce the number of queries further, the quick and sloppy way would be to replace the source parameter with sources, which would return the data we want but might also include some (source, signal) combinations we didn't intend (since signal names are not unique to their sources). The correct but complicated way would be to specify (source, signal) pairs. This is not a straight parameter promotion from a single value to a list; it's dropping two single-valued parameters and adding a new list-of-lists parameter. In the SQL we'd need to replace the two WHERE clauses with a conjunction-of-disjunctions clause. This is certainly possible, but it would be brand-new code rather than dropping in existing well-tested utility functions. Backwards-compatibility would require brand-new code, or we'd have to make it a breaking change. None of that is fast, particularly without a staging environment for testing API modifications.

@krivard krivard added the Triage Nominate for inclusion in the next release label Jul 8, 2020
@krivard krivard added this to the v1.6 milestone Jul 10, 2020
@krivard krivard removed the Triage Nominate for inclusion in the next release label Jul 14, 2020
@krivard
Copy link
Contributor Author

krivard commented Jul 14, 2020

Robin and Mike prefer Option 2, so we'll go with that.

krivard added a commit to krivard/delphi-epidata that referenced this issue Jul 30, 2020
…ators#116

* backwards-compatible signal/signals parameter
* optional 'format' parameter for retrieving results as a tree (preferred by Viz) or as a flat list (backwards-compatible); defaults to flat list
* client support: coffee, js, R, py
* integration tests to check multi-signal queries and queries for tree format
@krivard
Copy link
Contributor Author

krivard commented Aug 10, 2020

This is waiting on #158 to be completed before we can merge.

@krivard krivard added the blocked This task is waiting for completion of another task label Aug 10, 2020
@krivard krivard removed the blocked This task is waiting for completion of another task label Sep 1, 2020
krivard added a commit to krivard/delphi-epidata that referenced this issue Sep 4, 2020
…ators#116

* backwards-compatible signal/signals parameter
* optional 'format' parameter for retrieving results as a tree (preferred by Viz) or as a flat list (backwards-compatible); defaults to flat list
* client support: coffee, js, R, py
* integration tests to check multi-signal queries and queries for tree format
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 a pull request may close this issue.

2 participants