Skip to content

98 vignette on parsnip and recipes #112

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

Merged
merged 34 commits into from
Sep 15, 2022

Conversation

ChloeYou
Copy link
Contributor

@ChloeYou ChloeYou commented Aug 2, 2022

Addressed feedback from last meeting.

@ChloeYou ChloeYou linked an issue Aug 2, 2022 that may be closed by this pull request
@ChloeYou ChloeYou marked this pull request as ready for review August 15, 2022 17:20
@ChloeYou ChloeYou requested a review from dajmcdon as a code owner August 15, 2022 17:20
Copy link
Contributor Author

@ChloeYou ChloeYou left a comment

Choose a reason for hiding this comment

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

Add to-dos.

@ChloeYou ChloeYou marked this pull request as draft August 24, 2022 18:11
@ChloeYou ChloeYou marked this pull request as ready for review August 26, 2022 21:17
@dajmcdon
Copy link
Contributor

@ChloeYou The framework you have here is really great. I made a few updates, most pretty minor. Though I also had to make a bunch of changes to some code.

  1. Can you read over it all?
  2. Take a look at the changes to layer_population_scaling(). I added an argument to deal with "cases per 100K population". Basically case_rate isn't per capita, but per 100K inhabitants. Can you proof the documentation changes I made?
  3. Can you make a similar modification to step_population_scaling().
  4. Check over the documentation for state_census. I decided to add it here rather than grabbing from covidcast.

Thank you!

@dajmcdon dajmcdon merged commit fc0af3f into frosting Sep 15, 2022
@dajmcdon dajmcdon deleted the 98-vignette-on-parsnip-and-recipes branch September 15, 2022 00:36
@ChloeYou
Copy link
Contributor Author

@ChloeYou The framework you have here is really great. I made a few updates, most pretty minor. Though I also had to make a bunch of changes to some code.

  1. Can you read over it all?
  2. Take a look at the changes to layer_population_scaling(). I added an argument to deal with "cases per 100K population". Basically case_rate isn't per capita, but per 100K inhabitants. Can you proof the documentation changes I made?
  3. Can you make a similar modification to step_population_scaling().
  4. Check over the documentation for state_census. I decided to add it here rather than grabbing from covidcast.

Thank you!

Thank you for reviewing and making edits! I'll aim to get these done by EoW. Thanks!

@ChloeYou
Copy link
Contributor Author

ChloeYou commented Sep 15, 2022

@ChloeYou The framework you have here is really great. I made a few updates, most pretty minor. Though I also had to make a bunch of changes to some code.

  1. Can you read over it all?
  2. Take a look at the changes to layer_population_scaling(). I added an argument to deal with "cases per 100K population". Basically case_rate isn't per capita, but per 100K inhabitants. Can you proof the documentation changes I made?
  3. Can you make a similar modification to step_population_scaling().
  4. Check over the documentation for state_census. I decided to add it here rather than grabbing from covidcast.

Thank you!

On the second point, proof the documentation change as in write a test case for the functionality? @dajmcdon

@ChloeYou
Copy link
Contributor Author

ChloeYou commented Sep 16, 2022

Created new PR in #137 to address remaining comments. @dajmcdon

The changes in the code, addition of the plot of density of mask wearing, and state_census documentation all look good to me! Thanks again for the thorough review!

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.

Vignette - more on parsnip and recipes
2 participants