Skip to content

Limit rows returned for correlations #704

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

Merged
merged 3 commits into from
Sep 13, 2021
Merged

Conversation

dshemetov
Copy link
Contributor

Addresses this comment #646 (comment).

Prerequisites:

  • Unless it is a documentation hotfix it should be merged against the dev branch
  • Branch is up-to-date with the branch to be merged with, i.e. dev
  • Build is successful
  • Code is cleaned up and formatted

Summary

Currently, the /correlations endpoint in covidcast bypasses the row limits requirements by passing the query directly to pd.read_sql_query. This PR abstracts row limiting from run_query and adds it to as_pandas. A test to make sure it works is provided.

There may be a possible issue where someone requests too many rows and the row limit leads to the two correlated time series being mismatched, which would likely lead to an Exception later in the correlation code. We could try and send an error message if the number of returned rows == MAX_RESULTS instead of proceeding, getting an error, and sending nothing.

* add row limit to the request before forming dataframe
* add tests
* wrote test to ensure row limits work
* add test magic so as_pandas can be called
Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

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

👍

@krivard krivard merged commit a9dac7f into dev Sep 13, 2021
@krivard krivard deleted the dshemetov/limit_correlations branch September 13, 2021 17:35
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