PBK Review of mortality survey

[zkamvar]

This is in reference to the mortality survey template

• [x] line 248: creation of study_data_cleaned is confusing when we then go back to examine study_data_raw in lines 391 and 455 https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L247-L248 https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L391 Alex: disagree - because actually want to be browsing the original when recoding

• [x] line 458: why pull instead of select https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L455-L458

• [x] line 464: Might be worth commenting what we mean by error_tolerance = 0.5 https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L462-L464 Alex: @zkamvar can you chuck me a little blurb on here for explaining the error tolerance bit pls - I could read it up but easier this way. Future Alex to past Alex: no need to add - the help file explains it pretty clearly.

• [x] line 537: may be worth mentioning that age_categories creates a factor variable with levels specified from small to large https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L537-L541 Alex: think this is something for the website / function help file

• [x] line 563 description is not very clear, perhaps something like it will order your age_category from smallest unit to largest i.e. months to years if those are the categories wouldn’t we also want to show how to make all the ages in years if we have a mixture of months and years in a variable - thinking about age pyramids with too many groups if included separately for months as well as years https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L563-L566 Alex: This was my initial point - but it was disagreed with. Stick with how it is now. Will try and rephrase explanation.

• [x] line 584: just for code simplification/reduction i.e. avoiding case_when/ifelse? https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L580-L584 Alex: yep - will add a comment to point that out.

• [x] line 601: this is a separate mutate so as not to confuse people i.e. not just tagged on as an additional action linked with line 600 to avoid confusion? https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L601-L603 Alex: yeah. wont add comment on this.

• [x] line 641,651 - may be good to specify that find_start_date will create new columns I know we figure out that out from the help file, but confusing at first https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L641-L657 Alex: will add comment.

• [x] line 683: any reason why the mutate style changed? from data <- data %>% mutate() to data <- mutate(data, …) https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L683-L686 Alex: no, will change to fit others.

• [x] line 858: shouldn’t the comment say that the strata was by health district https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L858 Alex: yes, will change.

• [x] line 915: is there an overview somewhere of all the functions available in SitRep, could be useful https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L915 Alex: will be on package websites - @zkamvar worth adding to r4epis website? [Is on the packagedown site]

• [x] line 979: confused about use of the q65_iq4 variable, as it relates to GPS point rather than household number. Are we using gps point instead of HH number? https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L979 Alex: dictionary was shit. (gps point and hhnum were same i think) - should be solved by new kobo dictionaries. Hhnum should be one used.

• [x] line 1032: change in style and now directly starting with group_by() rather than the data frame - I know it doesn't change anything but it may throw newbies if style changes a lot https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1032 Alex: will change to fit style.

• [x] line 1040: any reason this wasn't saved as an object? Just makes it easier to review the steps of the process https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1040 Alex: would just be an extra step in an already dense chunk. Would argue to leave as is.

• [x] line 1053: what do we mean by many? https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1053 Alex: like more than a hundred. ball park. Will leave simple as the specifics just set unrealistic expectations and are all up for debate.

• [x] line 1096: why collapse == "--" instead of just "-"? Just so not interpreted as a minus? https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1096 Alex: yeah creates the right kind of hyphen. Wont comment.

• [x] line 1131: just a minor fyi - due to random data dictionary generator, we get pregnant babies! https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1131 Alex: could fix in gen_data but not really a priority.

• [x] line 1245: the show_halfway is included by default, not sure it needs to be. Might be worth explaining this in comment, but yes, they can read the help file https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1245

• [x] line 1331: may be worth naming the ci variable in output as prop_ci https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1331 Alex: not against it. @zkamvar any objection? Although would involve changing a lot of different bits of code and it doesnt really add much - so actually im half against doing this.

• [x] line 1468: any reason we don't just order based on the proportion? https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1468 Alex: no but also no particular reason to choose one over the other.

• [x] line 1474: worth adding a line to fix the 95% CI, as negative with my dataset https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1474 Alex: @zkamvar does tab_survey change negative CIs to zero? Maybe we should add a line here to do that as survey_ratio is part of {srvyr}

• [x] line 1477: could consider ordering by proportion and fixing the 95% CI https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1477 Alex: not re-ordering, but potentially fix CIs depending on response to above point.

• [x] line 1728: are these flow tables routinely presented in mortality surveys? https://github.com/R4EPI/sitrep/blob/9b44e408e70689a451d4062f109f6aefb670e7f2/inst/rmarkdown/templates/mortality/skeleton/skeleton.Rmd#L1728 Alex: no, but leave as is - can be deleted if people dont like.

• [x] In all tempaltes replace SYMPTOMS for any_of(SYMPTOMS) when trying to feed multiple vars to a function. See https://github.com/R4EPI/epibuffet/pull/9#issuecomment-584204086

[zkamvar]

line 458: why pull instead of select

This is because we want a single vector, not a one-column data frame: https://dplyr.tidyverse.org/reference/pull.html

[zkamvar]

line 1245: the show_halfway is included by default, not sure it needs to be. Might be worth explaining this in comment, but yes, they can read the help file

If I understand correctly, you are suggesting two things:

1. set show_halfway to default to FALSE
2. explain show_halfway in the comments

Can you elaborate a bit more about why show_halfway should not be included by default?

[pbkeating]

@zkamvar thanks for this. I haven't seen the output of show_halfway = TRUE as standard in age-sex pyramids that I've seen presented. I think it's good to have, but perhaps better to default to FALSE.

@aspina7 Many of my above comments were general comments rather than major bugs that need fixing. Useful to do the same for the other templates?

[aspina7]

yeap - id say so. Churn through em

[zkamvar]

@zkamvar thanks for this. I haven't seen the output of show_halfway = TRUE as standard in age-sex pyramids that I've seen presented. I think it's good to have, but perhaps better to default to FALSE.

Is there a reason beyond "this is always how we've done it?".

[pbkeating]

No

On Mon, 16 Dec 2019, 09:41 Zhian N. Kamvar, notifications@github.com wrote:

@zkamvar https://github.com/zkamvar thanks for this. I haven't seen the output of show_halfway = TRUE as standard in age-sex pyramids that I've seen presented. I think it's good to have, but perhaps better to default to FALSE.

Is there a reason beyond "this is always how we've done it?".

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/R4EPI/sitrep/issues/221?email_source=notifications&email_token=AE2NEQ72X63ZSCTTJXQUSEDQY5ELHA5CNFSM4JZNQSV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG6DIJA#issuecomment-565982244, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE2NEQ6LMRS6WCMLFOIJA5DQY5ELHANCNFSM4JZNQSVQ .

[zkamvar]

Unless it causes a lot of confusion, I'll leave it TRUE by default since it provides a non-intrusive visual aid and we can write the comment to explicitly exclude it (which will be easier to write than asking people to include something they've never seen before).

[aspina7]

Unless it causes a lot of confusion, I'll leave it TRUE by default since it provides a non-intrusive visual aid and we can write the comment to explicitly exclude it (which will be easier to write than asking people to include something they've never seen before).

Agree with wizard

[aspina7]

line 1728: are these flow tables routinely presented in mortality surveys?

No - previously was ugly tables .... felt like it was an easier overview. Disagree?

[pbkeating]

It's fine by me, but just depends on the audience, I guess.

On Wed, 18 Dec 2019 at 10:06, Alex Spina notifications@github.com wrote:

line 1728: are these flow tables routinely presented in mortality surveys?

No - previously was ugly tables .... felt like it was an easier overview. Disagree?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/R4EPI/sitrep/issues/221?email_source=notifications&email_token=AE2NEQYRIN7P7EFYYQJRWSLQZHYZJA5CNFSM4JZNQSV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHFTBWQ#issuecomment-566964442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE2NEQZEZ4L36H5K4COFMBDQZHYZJANCNFSM4JZNQSVQ .