Skip to content
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

New agg_tf function for calculating total fertility #70

Merged
merged 9 commits into from
Dec 28, 2020
Merged

New agg_tf function for calculating total fertility #70

merged 9 commits into from
Dec 28, 2020

Conversation

chacalle
Copy link
Collaborator

Describe changes

This adds a new function agg_tf for calculating total fertility measures. This builds off of an old PR from Claudia's fork #55. I mainly just resolved some remaining comments from that first PR and updated to the newer hierarchyUtils::agg arguments.

What issues are related

Addressed remaining comments from previous PR

Related to #19

Checklist

Packages Repositories

  • Have you read the contributing guidelines for ihmeuw-demographics R packages?
  • Have you successfully run devtools::check() locally?
  • Have you updated or added function (and vignette if applicable) documentation? Did you update the 'man' and 'NAMESPACE' files with devtools::document()?
  • Have you added in tests for the changes included in the PR?
  • Do the changes follow the ihmeuw-demographics code style?
  • Do the changes need to be immediately included in a new build of docker-base or docker-internal? If so follow directions in those repositories to rebuild and redeploy the images.
  • Do the changes require updates to other repositories which use this package? If yes, make the necessary updates in those repos, and consider integration tests for those repositories.

Details of PR

None

@chacalle chacalle added the enhancement New feature or request label Dec 21, 2020
@chacalle chacalle self-assigned this Dec 21, 2020
krpaulson
krpaulson approved these changes Dec 24, 2020
View reviewed changes
Copy link
Collaborator

@krpaulson krpaulson left a comment

Choose a reason for hiding this comment

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

Looks good to me!

R/agg_tf.R
#' fertility rates and (2) she survives through the end of the age span.
#' Preston pg 95.
#'
#' This is different from an age-specific fertility rate (ASFR) or a crude birth
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is helpful.

tests/testthat/test_agg_tfr.R
@@ -0,0 +1,37 @@
library(data.table)

# test dataset from Preston pg 96 Box 5.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good idea

@chacalle chacalle merged commit 94c1dc0 into master Dec 28, 2020
@chacalle chacalle deleted the feature/tfr branch December 28, 2020 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krpaulson krpaulson krpaulson approved these changes

@erinamay erinamay Awaiting requested review from erinamay

@hcomfo95 hcomfo95 Awaiting requested review from hcomfo95

Assignees

@chacalle chacalle

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chacalle @krpaulson