-
Notifications
You must be signed in to change notification settings - Fork 67
Add async method to epidata #396
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
Conversation
krivard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks legit, but the documentation for delphi_epidata is so nonexistant that we definitely need people to be able to tell what get/fetch are for just by their names. No accidents from people doing dir(Epidata) and guessing
(do get/fetch even need to be static members or can they be local functions within async_call? will you be calling them directly from nowcast?)
Co-authored-by: Katie Mazaitis <krivard@cs.cmu.edu>
Co-authored-by: Katie Mazaitis <krivard@cs.cmu.edu>
Co-authored-by: Katie Mazaitis <krivard@cs.cmu.edu>
Co-authored-by: Katie Mazaitis <krivard@cs.cmu.edu>
Good point, only the async_call is the only one that will be used publicly (maybe i should switch the names of async_call and async_fetch_epidata), so the others don't need to be visible. Ill work on it |
|
Updated to make functions local and also renamed them. |
krivard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👩🍳 💋
To be used by covidcast client (to speed up indicator combo) and also in nowcasting.
One thing to note is that running this on some machines and/or in jupyter can lead to this an issue that requires you run these two lines to solve. For that reason I'm not sure we'll want to make this the default method in covidcast, since we don't have much control over people's environments. Adding those lines at the top of the client file seems to resolves it for me, but not sure if that'll hold universally. If it does, then we might be fine. If not, we could set a protected flag in covidcast for async that only our internal users/superusers will change.
Summary of changes: