Skip to content
This repository has been archived by the owner on Jul 7, 2022. It is now read-only.

Altair #58

Merged
merged 7 commits into from
Mar 17, 2020
Merged

Altair #58

merged 7 commits into from
Mar 17, 2020

Conversation

quinn-dougherty
Copy link
Contributor

@quinn-dougherty quinn-dougherty commented Mar 17, 2020

https://github.com/pennsignals/chime/issues/43

  • factored out matplotlib
  • charts are now calls to pure functions
  • current interactivity is zoom in/out & scroll side to side, mouseover tooltip showing values

@quinn-dougherty
Copy link
Contributor Author

i do not have https://github.com/pennsignals/chime/issues/29 in here i actually think that should be a separate PR after this one

app.py Outdated Show resolved Hide resolved
@mdbecker
Copy link
Member

mdbecker commented Mar 17, 2020

Looks like there's a regression in the census graph when I change some of the input values
Input values I tested with to produce this issue:

  • Currently Known Regional Infections: 100
  • Currently Hospitalized COVID-19 Patients: 20
  • Doubling Time (days): 12
    Old:
    image

New:
image
Both the y and x axis seem off

@jlubken
Copy link
Member

jlubken commented Mar 17, 2020

Add the altair dependency to requirements.txt, and environment.yml too. Remove matplotlib if it is no longer needed.

@quinn-dougherty
Copy link
Contributor Author

Add the altair dependency to requirements.txt, and environment.yml too. Remove matplotlib if it is no longer needed.

done. thanks @jlubken!

@quinn-dougherty
Copy link
Contributor Author

Screenshot from 2020-03-17 12-55-14
@mdbecker this is what i got currently. correct numbers but less granularity

  • Currently Known Regional Infections: 100
  • Currently Hospitalized COVID-19 Patients: 20
  • Doubling Time (days): 12

do you want to merge now or wait until i crack this?

the thing is, this whole block might need to be refactored

for k, los in los_dict.items():
    census = (
        projection_admits.cumsum().iloc[:-los, :]
        - projection_admits.cumsum().shift(los).fillna(0)
    ).apply(np.ceil)
    census_dict[k] = census[k]

census_df = pd.DataFrame(census_dict)
census_df["day"] = census_df.index
census_df = census_df[["day", "hosp", "icu", "vent"]]

census_table = census_df[np.mod(census_df.index, 7) == 0].copy()
census_table.index = range(census_table.shape[0])
census_table.loc[0, :] = 0
census_table = census_table.dropna().astype(int)

to pull out the census[k]s into a solitary dataframe. I don't want to add too much complexity and force yall to verify the model correctness of a PR if i don't have to

@cjbayesian
Copy link
Contributor

This is great. I am looking at it and is giving me the right numbers. @quinn-dougherty can you help me understand why the granularity is lower for the census plots?

@cjbayesian
Copy link
Contributor

cjbayesian commented Mar 17, 2020

Ahh, I see I think. You can plot census_df instead of census_table .

@quinn-dougherty
Copy link
Contributor Author

This is great. I am looking at it and is giving me the right numbers. @quinn-dougherty can you help me understand why the granularity is lower for the census plots?

line 264 of my branch census_table = census_df[np.mod(census_df.index, 7) == 0].copy() suggests to me that the correct label is weeks from today, but in the original it's days from today.

I got the correct numbers by passing census_table into my chart building function, and i get incorrect numbers by passing either census_df or census into it. (passing census was clearly incorrect and i shouldn't have had that in my first draft). The trouble is due to the mod operation there's less data in census_table.

the original called ax.plot(census.head(plot_projection_days)[k], ".-", label=k + " census") from inside the for loop, but it's not fully clear to me why it results in something different than building a chart off of census_df.

@quinn-dougherty
Copy link
Contributor Author

Ahh, I see I think. You can make plot census_df instead of census_table .

calling my chart build func on census_df gives the right granularity and the wrong numbers.

@cjbayesian
Copy link
Contributor

cjbayesian commented Mar 17, 2020

Trying to figure it out now. I suggest we go with the 7 day granularity for now to get the stability and submit an issue to fix the granularity.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants