Skip to content

C2B Post for review #256

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 56 commits into from
Mar 31, 2021
Merged

C2B Post for review #256

merged 56 commits into from
Mar 31, 2021

Conversation

@Ananya-Joshi
Copy link
Contributor

@Ananya-Joshi Ananya-Joshi commented Mar 9, 2021

C2B post in markdown format to review

  • Are the images of good quality
  • Do the captions show in markdown
  • Any additional comments about the post.

@sgratzl sgratzl mentioned this pull request Mar 10, 2021
Closed
@sgratzl
Copy link
Member

sgratzl commented Mar 10, 2021

I migrated this PR to #257 which is following the blog template

sgratzl and others added 2 commits March 10, 2021 09:32
@sgratzl
style(fix): fix modal image size
d939da4
@Ananya-Joshi
Fixed ToDos
86e0fb0 
Hero Images
Author Descriptions
Tags
Acknowledgements
@Ananya-Joshi Ananya-Joshi deleted the c2b_blogpost branch March 10, 2021 17:02
@Ananya-Joshi Ananya-Joshi reopened this Mar 10, 2021
@netlify
Copy link

netlify bot commented Mar 10, 2021 •
edited
Loading

Deploy preview for cmu-delphi-main ready!

Built with commit 49c3074

https://deploy-preview-256--cmu-delphi-main.netlify.app

@Ananya-Joshi
Copy link
Contributor Author

Ananya-Joshi commented Mar 10, 2021 •
edited
Loading

Thank you @sgratzl for your comments!

sgratzl
sgratzl requested changes Mar 11, 2021
View reviewed changes
Copy link
Member

@sgratzl sgratzl left a comment

Choose a reason for hiding this comment

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

please ensure the proper format and size of the hero images:

  • large image: 1120x440 in both jpg and webp format - currently: 476x200 in a renamed png format
  • thumbnail: 300x200 in both jpg and webp format - currently: 300x200 in a renamed png format

@tildechris
Copy link
Contributor

tildechris commented Mar 31, 2021

@Ananya-Joshi the new plots look great! A couple of notes:

Hospitalizations and Cases

image

  1. You can keep the key colors but hide the key (show.legend = FALSE or guides(color = FALSE)) since you don't have both on one plot anymore.
  2. Missing source attribution. This is from the WPRDC?
  3. You don't need date ranges in the title. If you wanted you could be more specific on the units e.g. "Hospitalizations due to COVID-19 and New cases from positive PCR tests" or similar.

Percent hospitalized

image

  • You mention in the copy age, sex and race in the copy. You should either update the copy ("varies most by age, and to a lesser extent sex and race") or re-include gender and race plots.
  • You can include new-line escapes to the caption (\n) to break the caption into one line for data attribution and one for the group.

Hospital Utilization

image

  • Again, no need for a legend here, but since you more precisely define the unit here, you should move the "7d-Avg" into the sub-title.
  • You don't need data attribution in the subtitle if you have it in the caption
  • You can break the data source and by-lines into different lines

@tildechris
Copy link
Contributor

tildechris commented Mar 31, 2021

@Ananya-Joshi one final suggestion: if you have a Date axis and it literally means calendar date (like in your estimation plots), you can omit the x-axis label. Roni had the suggestion, however, that we can be more precise in x-axis by saying "reported date" (from COVIDcast or similar) or whatever the date label is from the source (e.g. collection date)

@sgratzl
Copy link
Member

sgratzl commented Mar 31, 2021

@Ananya-Joshi please merge the branch sgratzl/c2b_blogpost_updates into this one. It should update you to the latest version and fix all the conflicts

@tildechris tildechris requested a review from sgratzl March 31, 2021 19:32
sgratzl
sgratzl reviewed Mar 31, 2021
View reviewed changes
content/blog/2021-03-10_c2b.Rmd Outdated
library("ggpubr")
library(scales)

full_df <- tibble(read.csv(file="../../public/blog/2021-03-10_c2b/test_and_hosp.csv"))
Copy link
Member

@sgratzl sgratzl Mar 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I thought I moved the files and updated the links in my update branch (sgratzl/c2b_blogpost_updates) ...

Copy link
Member

@sgratzl sgratzl Mar 31, 2021

Choose a reason for hiding this comment

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

see https://github.com/cmu-delphi/www-main/tree/sgratzl/c2b_blogpost_updates/content/blog/c2b

Copy link
Contributor Author

@Ananya-Joshi Ananya-Joshi Mar 31, 2021

Choose a reason for hiding this comment

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

Hm I wonder what happened here, I'll undo my last commit

Copy link
Contributor

@tildechris tildechris Mar 31, 2021

Choose a reason for hiding this comment

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

Merged. Please take a look.

@tildechris tildechris requested a review from sgratzl March 31, 2021 20:21
sgratzl
sgratzl approved these changes Mar 31, 2021
View reviewed changes
Copy link
Member

@sgratzl sgratzl left a comment

Choose a reason for hiding this comment

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

👍

@tildechris tildechris merged commit 4010d73 into cmu-delphi:dev Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@capnrefsmmat capnrefsmmat capnrefsmmat left review comments

@sgratzl sgratzl sgratzl approved these changes

+2 more reviewers

@tildechris tildechris tildechris left review comments

@bweaver-work bweaver-work bweaver-work left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

blog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Ananya-Joshi @sgratzl @tildechris @capnrefsmmat @bweaver-work