Skip to content

Avoid filter(cases==1), better describe & complete Ebola example #260

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 2 commits into from
May 29, 2024

Conversation

brookslogan
Copy link
Contributor

Avoid filter(cases==1): this should either

  • involve filter(status == "confirmed") instead, or
  • involve no filter, and let sum take care of things.

We should generally expect the latter approach to be RAM-friendlier (one new column vs. new filtered vectors for every column), so implement the latter approach.

Reformat the code a bit to try to make it read better.

Clarify the description of the example; make clear we're deriving from line list data.

complete the data to make the set of time_values evenly spaced and the same for every geo_value; even though it doesn't impact the plots, we're demonstrating how to get things into epi_df format, and many functions we'd use on epi_dfs will expect things in this format.

@brookslogan brookslogan requested a review from dajmcdon as a code owner January 20, 2023 21:32
Copy link
Contributor

@dajmcdon dajmcdon left a comment

Choose a reason for hiding this comment

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

Feel free to ignore my suggestion. Everything else appears fine. Merge when ready.

@brookslogan brookslogan changed the base branch from main to dev June 21, 2023 17:35
@brookslogan brookslogan changed the base branch from dev to main June 21, 2023 17:35
@brookslogan brookslogan force-pushed the lcb/clarify-ebola-example-code branch from 15a6023 to b0c2317 Compare June 21, 2023 20:46
@dsweber2 dsweber2 added this to the Epiprocess Issue Triage milestone Dec 6, 2023
@brookslogan brookslogan force-pushed the lcb/clarify-ebola-example-code branch from b0c2317 to d14ee3f Compare May 28, 2024 23:03
@brookslogan
Copy link
Contributor Author

Finally getting around to revisiting this. I thought there was something else to do here, but apparently there was only styling left? I've held off on converting things to use case_match() until we want dplyr 1.1.0 functionality for something more important, as potential encoding breaking changes and summarize() complaint spam may have people holding off.

Avoid `filter(cases==1)`: this should either

* involve `filter(status == "confirmed")` instead, or
* involve no filter, and let `sum` take care of things.

We should generally expect the latter approach to be RAM-friendlier (one new
column vs. new filtered vectors for every column), so implement the latter
approach.

Reformat the code a bit to try to make it read better.

Clarify the description of the example; make clear we're deriving from line list
data.

`complete` the data to make the set of `time_value`s evenly spaced and the same
for every `geo_value`; even though it doesn't impact the plots, we're
demonstrating how to get things into `epi_df` format, and many functions we'd
use on `epi_df`s will expect things in this format.
@brookslogan brookslogan force-pushed the lcb/clarify-ebola-example-code branch from d14ee3f to 9417d33 Compare May 29, 2024 17:34
@brookslogan brookslogan merged commit d89c14c into dev May 29, 2024
4 checks passed
@brookslogan brookslogan deleted the lcb/clarify-ebola-example-code branch May 29, 2024 17:35
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.

3 participants