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

Refactor indicators to use delphi_utils library calls #306

Closed
krivard opened this issue Oct 9, 2020 · 14 comments
Closed

Refactor indicators to use delphi_utils library calls #306

krivard opened this issue Oct 9, 2020 · 14 comments
Assignees
Labels
Engineering Used to filter issues when synching with Asana

Comments

@krivard
Copy link
Contributor

krivard commented Oct 9, 2020

We now/will soon have the following pipeline utilities in delphi_utils:

These behaviors are currently implemented individually in each indicator. This ticket will track refactors on all indicators to use the library versions instead.

indicator geo smoothing validation
cdc_covidnet in progress (@chinandrew)
cds[1]
claims_hosp in progress (@chinandrew)
combo_cases_and_deaths ✅ (no change necessary)
doctor_visits in progress (@chinandrew)
emr_hosp[2]
facebook[3]
google_health[4]
jhu in progress
nchs_mortality
quidel
quidel_covidtest
safegraph in progress (@chinandrew)
safegraph_patterns
usafacts in progress (@sgsmob)

[1] - we don't plan to continue development on CDS; don't refactor it
[2] - the emr-hosp indicator is no longer active; low priority
[3] - the facebook indicator is in R; python utilities can't be a part of the main computation (smoothing, aggregation) but validation can be added as a separate step
[4] - deprecated in favor of a new indicator using the new symptoms search product; don't refactor it

@krivard
Copy link
Contributor Author

krivard commented Oct 13, 2020

@chinandrew
Copy link
Contributor

I can take the covidnet geo refactor. Updated table accordingly

@chinandrew
Copy link
Contributor

I believe for Google Health we need DMA methods with aren't available yet, so made an issue and updated the table. Going down the list, will take safegraph next (which is also a one liner since all it's doing is mapping state abbr and FIPS with a dict, which honestly isn't the worst -- just not consistent)

@dshemetov
Copy link
Contributor

@chinandrew We decided to avoid implementing DMA methods because of difficulty and the deprecation of the Google Health indicator.

@krivard
Copy link
Contributor Author

krivard commented Oct 15, 2020

@nmdefries do you think you'll have a drop-in replacement from Google Symptoms available quicker than we'd be able to do a smoothing refactor in the old code? if so we might as well remove the old google health indicator from this list entirely.

@nmdefries
Copy link
Contributor

It's not clear to me how long it takes to spin up a new indicator, but GHT has other issues (bag of search terms might need to be re-chosen, #292; high variability; lack of documentation) that could make it not worth updating if we're ultimately going to move to Google Symptoms.

Tagging @huisaddison for more context.

@krivard
Copy link
Contributor Author

krivard commented Oct 15, 2020

We already decided not to update the GHT search terms, so that's out regardless.

The question then is how close are you in GS to a widget that produces COVIDcast CSV files? days, weeks?

@nmdefries
Copy link
Contributor

Got it.

I think it would take me a bit to get up to speed on indicator package construction - I've been working on EDA, data quality, basic forecasting with GS. I can't give a super accurate estimate, but I'd guess the widget would take a couple weeks.

@sgsmob
Copy link
Contributor

sgsmob commented Oct 16, 2020

What is "doctor_visits"?

@chinandrew
Copy link
Contributor

What is "doctor_visits"?

I asked @krivard the same thing, it's the code in here #76

@SumitDELPHI SumitDELPHI added the Engineering Used to filter issues when synching with Asana label Dec 6, 2020
@chinandrew
Copy link
Contributor

Looking at the code, it appears quidel isnt refactored yet, but quidel_covidtest is. I see the checkbox for quidel is just an Oct 14th edit without a linked PR, so not sure what the history of that is. @krivard do you recall any details around quidel?

@krivard
Copy link
Contributor Author

krivard commented Jan 4, 2021

@chinandrew there is a PR to merge quidel and quidel_covidtest, which in theory uses geo for both. That PR changes the way thresholding works and drops a bunch of borderline regions, so merging it is blocked on deletion encoding.

@chinandrew
Copy link
Contributor

Are you referring to #181? Don't see any other quidel PRs aside from #665 which I opened a few days ago

@krivard
Copy link
Contributor Author

krivard commented Jan 4, 2021

hmmm, I guess it's not a PR, it's the run-quidel branch

@krivard krivard closed this as completed Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Used to filter issues when synching with Asana
Projects
None yet
Development

No branches or pull requests

7 participants