-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
[NM-99] Return all indicators for input time series #456
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good.
See suggestions on the labels. @rtesra can add or suggest others.
@@ -1,3 +1,7 @@ | |||
# naomi 2.10.3 | |||
|
|||
* Return `anc_already_art`, `anc_status`, `anc_art_among_known` and `anc_total_pos` indicators from ANC input time series data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is anc_status
? Is that the total denominator? I usually use anc_hiv_status
for that. Doesn't matter too much
anc_prevalence, anc_known_pos, anc_known_neg, | ||
anc_art_coverage, births_facility, births_clients_ratio, | ||
area_hierarchy) |> | ||
dplyr::select(-sex) |> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any risk that anc_long
has other columns dangling around, or are the columns of that table rigidly specified by an earlier step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the indicators are specified by mutate_exprs. So not selecting specific columns here means if we add any more then they should come straight through. Are you worried about some new non-indicator column though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double checked, no that shouldn't happen. We read the anc file in and specify the columns we want to read. So that should filter out any unwanted stuff. This should just mean all the columns we create during the aggregation get returned automatically
inst/traduire/en-translation.json
Outdated
"ANC_TOTAL_POS": "Anc total pos", | ||
"ANC_TOTAL_POS_DESC": "Anc total pos desc", | ||
"ANC_ALREADY_ART_DESC": "ANC already ART desc", | ||
"ANC_STATUS": "ANC status", | ||
"ANC_STATUS_DESC": "ANC status desc", | ||
"ANC_ART_AMONG_KNOWN": "ANC ART among known", | ||
"ANC_ART_AMONG_KNOWN_DESC": "ANC ART among known desc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ANC total HIV positive"
"Total HIV positive women at ANC (known positive and tested positive)"
"Women already on lifelong ART before first ANC visit during this pregnancy"
"ANC HIV status ascertained"
"Number of women with HIV status ascertained during ANC (either known or tested at least once)"
"ANC ART coverage among known positive" this might be too long?
"Proportion of women known HIV positive at first ANC visit who were already on ART"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good — thanks
No description provided.