Skip to content

Conversation

fbenamy
Copy link
Collaborator

@fbenamy fbenamy commented Mar 18, 2020

Closes #11

@jlubken jlubken self-requested a review March 18, 2020 19:42
@cjbayesian
Copy link
Collaborator

Please merge in latest master in order to allow a side-by-side comparison of numbers with current prod.

@jlubken jlubken removed their request for review March 18, 2020 19:53
@fbenamy
Copy link
Collaborator Author

fbenamy commented Mar 18, 2020

@cjbayesian Just merged the current master into this branch.

@cjbayesian
Copy link
Collaborator

cjbayesian commented Mar 18, 2020

Thanks, @fbenamy . This looks like it's going to work, but there is code currently in this branch that is not the latest in terms of how the initial infected population is being calculated. Specifically, this input should not be there:
image

All of the code that is not related to just the census chart should not differ between this and master.

@fbenamy
Copy link
Collaborator Author

fbenamy commented Mar 18, 2020

That's very weird. I pulled from the Code for Philly master before merging. Let me do some digging and see what might be up with my local repo not staying current.

@fbenamy
Copy link
Collaborator Author

fbenamy commented Mar 18, 2020

So it looks like I somehow removed app.py from the merge commit. I just checked that the code is right in the online repo.

Copy link
Collaborator

@quinn-dougherty quinn-dougherty left a comment

Choose a reason for hiding this comment

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

hi -- the sidebar input order doesn't agree with prod. Unless there's an explanation for this i don't know about, could you please align the sidebars? they should look exactly the same

Copy link
Collaborator Author

@fbenamy fbenamy left a comment

Choose a reason for hiding this comment

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

I just updated the code. My apologies for all the back and forth errors. I just recently changed ides and managed to bungle the handling of merge conflicts.

Copy link
Collaborator

@quinn-dougherty quinn-dougherty left a comment

Choose a reason for hiding this comment

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

perfect, thanks!

@fbenamy fbenamy merged commit ec420cd into master Mar 19, 2020
@fbenamy fbenamy deleted the graph-granularity branch March 19, 2020 00:33
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.

admitted patients (census) chart is weekly granular, should be daily
3 participants