cmu-delphi / www-covidcast

Front end for interactive visualizations powering the COVIDcast website.
https://delphi.cmu.edu/covidcast/
MIT License
13 stars 2 forks source link

hospital admission county override #1164

Closed sgratzl closed 2 years ago

sgratzl commented 2 years ago

closes #1139

Prerequisites:

Summary

option to override a signal with another one in the config for a specific geo level. The override is then patched in in both fetching the data and fetching the trend.

what is missing is adapting the text to notify the user about it at https://docs.google.com/document/d/1llv6xh8jMlmVv7WpyDSv4VgUpFAZQ6QjFUeRxxOinmk/edit#

netlify[bot] commented 2 years ago

Preview link ready!

Name Link
Latest commit 420164a605fc026d59c61603deb1ba29babfd848
Latest deploy log https://app.netlify.com/sites/cmu-delphi-covidcast/deploys/6271b810dd6bd60009f69293
Deploy Preview https://deploy-preview-1164--cmu-delphi-covidcast.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

ryantibs commented 2 years ago

Thanks for the fast turnaround. Unfortunately some things don't seem to work for me:

Screen Shot 2022-02-27 at 8 55 12 PM Screen Shot 2022-02-27 at 8 55 43 PM Screen Shot 2022-02-27 at 8 56 10 PM Screen Shot 2022-02-27 at 8 56 42 PM
sgratzl commented 2 years ago

time series map showing very weird pattern of availability at the county level

that is what the data seems to look like, see also https://api.covidcast.cmu.edu/epidata/covidcast/?signal=dsew-cpr%3Aconfirmed_admissions_covid_1d_prop_7dav&geo=county%3A42003&time=day%3A20220129-20220226&format=json&fields=value%2Cstderr%2Cgeo_type%2Cgeo_value%2Ctime_value

e.g. the last two entries:

image

which also explains the other observed behavior.

krivard commented 2 years ago

We're working on an interpolation fix to fill the gap at pipeline-time; this PR may need to be kept on hold until that is complete.

ryantibs commented 2 years ago

@dshemetov @korlaxxalrok When the interpolation fix for the county-level hospitalization signal is live, can one of you check the preview ("Browse the preview") to see that it's gone through as expected? (I checked just now and it hasn't yet.)

Once that's done, you could ping @sgratzl and I to confirm on this, and then I could look once more before we merge.

cc @duanecmu to help track.

dshemetov commented 2 years ago

@ryantibs county-level interpolation is live and looks good. Slack thread for the release.

ryantibs commented 2 years ago

Thanks! I played around with the main dashboard a bit and it looks good to me there as well.

@dshemetov @duanecmu @korlaxxalrok Can one of you play around with this between county, state, and national levels on the main dashboard as well to see that it all works are you expect?

Also, can you confirm: the earliest we have the county-level hospitalization data (from CPR) for is Dec 2020 or so? Because we have state and national hospitalization data (from HHS) going back earlier. See screenshot:

Screen Shot 2022-05-03 at 5 16 50 PM
ryantibs commented 2 years ago

Also I realized that we don't describe (in the gray box data description box) the fact that at the county level, we're showing data from CPR. I believe it still says it's from HHS. We should fix this.

@krivard Can you please fix and/or delegate? Thank you!

Then, once fixed, and I hear back that at least one other person played with this staging dashboard, then I'm fine to merge (Katie you can approve yourself---it looks like it's on auto merge as soon as it's approved, so I won't approve yet).

krivard commented 2 years ago

Also, can you confirm: the earliest we have the county-level hospitalization data (from CPR) for is Dec 2020 or so?

The earliest Community Profile Report is dated 2020-12-17, but it does not include county-level hospital admissions. The first CPR including county-level hospital admissions is dated 2021-01-08, and includes the admissions count for the 7-day period ending 2021-01-07.

I checked all geo levels and it looks good:

ryantibs commented 2 years ago

(Great---thank you! So I think just the description is left.)

ryantibs commented 2 years ago

I'm going to go ahead and approve this since IIRC the description gets updated through a separate workflow.

sgratzl commented 2 years ago

@krivard I'm not sure what the current status of this PR is

ryantibs commented 2 years ago

(Thank you for checking Sam, I was also about to check on it right now!)

duanecmu commented 2 years ago

Just wanted to check to see if there were any status updates for this PR. Thanks!

krivard commented 2 years ago

@duanecmu you should be able to see the status updates: image

This PR has been merged and will be deployed at the next release of www-covidcast. Would you like to do the honors?

duanecmu commented 2 years ago

Sure thing. I'll start the deployment today if that's OK.