Skip to content

Conversation

nmdefries
Copy link
Collaborator

@nmdefries nmdefries commented Apr 17, 2023

  • Make select dplyr-based processing steps simpler and faster
  • Remove unnecessary pipes
  • Use data.table's character vector-optimized "in" operator, %chin%, in slow filter steps
  • Use data.table's wday function to get weekday name from dates

@nmdefries nmdefries marked this pull request as ready for review April 17, 2023 20:11
@nmdefries nmdefries requested a review from krivard April 17, 2023 20:11
Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

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

Couple of confirmations but otherwise good to go 🚀

Comment on lines +61 to +67
scoreDf$ahead_group <- case_when(
scoreDf$ahead >= HOSPITALIZATIONS_OFFSET & scoreDf$ahead < 7 + HOSPITALIZATIONS_OFFSET ~ 1L,
scoreDf$ahead >= 7 + HOSPITALIZATIONS_OFFSET & scoreDf$ahead < 14 + HOSPITALIZATIONS_OFFSET ~ 2L,
scoreDf$ahead >= 14 + HOSPITALIZATIONS_OFFSET & scoreDf$ahead < 21 + HOSPITALIZATIONS_OFFSET ~ 3L,
scoreDf$ahead >= 21 + HOSPITALIZATIONS_OFFSET & scoreDf$ahead < 28 + HOSPITALIZATIONS_OFFSET ~ 4L,
TRUE ~ NA_integer_
)
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: this is a beautiful refactor

filteredScoreDf[, c("Forecaster", "Forecast_Date", "Week_End_Date", "Score", "ahead")],
!is.na(Week_End_Date)
)
filteredScoreDf$Score <- round(filteredScoreDf$Score, 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

extra explanation for future travelers:

This change is equivalent.

Score is the only column where rounding to 2 decimal places will have any effect.

ahead is numeric but takes integer values.

@nmdefries nmdefries merged commit 49896af into dev Apr 19, 2023
@nmdefries nmdefries deleted the ndefries/cleanup-dplyr-in-usage branch April 19, 2023 20:56
@krivard krivard mentioned this pull request Apr 26, 2023
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.

2 participants