Skip to content

Added update.layer and update_frosting #221

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 39 commits into from
Oct 25, 2023
Merged

Added update.layer and update_frosting #221

merged 39 commits into from
Oct 25, 2023

Conversation

rachlobay
Copy link
Contributor

@rachlobay rachlobay commented Jul 21, 2023

Addresses #73. Added update.layer() and update_frosting() along with tests for each.

@rachlobay rachlobay requested a review from dajmcdon as a code owner July 21, 2023 16:47
@rachlobay rachlobay linked an issue Jul 21, 2023 that may be closed by this pull request
@rachlobay
Copy link
Contributor Author

Actually, hold up... I see that workflows update_recipe() and remove_recipe() convert the epi_workflow to just a workflow unfortunately... And I expect that their update_model() function will have a similar result... I will have to add the equivalents to work with an epi_workflow(). It's probably best to try get this done all on one PR (as they are very similar tasks), so I will let you know/post here after I've done this.

@rachlobay
Copy link
Contributor Author

rachlobay commented Jul 24, 2023

Update: I’ve finished adding functions (+ tests) for add/update/remove a model and a recipe that work with an epi_workflow. Note that I did not add functions to update a recipe step or model specification in a parsnip model because the existing functions for those seem to work ok.

What I mean by the last sentence can be seen through the following example:

library(epiprocess)
library(dplyr)
library(recipes)

jhu <- case_death_rate_subset %>%
  filter(time_value > "2021-08-01") %>%
  dplyr::arrange(geo_value, time_value)

r <- epi_recipe(jhu) %>%
  step_epi_lag(death_rate, lag = c(0, 7, 14)) %>%
  step_epi_ahead(death_rate, ahead = 7) %>%
  step_epi_lag(case_rate, lag = c(0, 7, 14)) %>%
  step_naomit(all_predictors()) %>%
  step_naomit(all_outcomes(), skip = TRUE)

# Update a recipe step using the tidymodels function
# https://github.com/tidymodels/recipes/blob/HEAD/R/update.R
r2 <- r
r2$steps[[1]] <- update(r2$steps[[1]], lag = 4)

# Update model specification for a parsnip model using the parsnip function
# https://parsnip.tidymodels.org/reference/parsnip_update.html
mod1 <- rand_forest(mode = "regression", trees = 10)

update(mod1, trees = 20)

@rachlobay rachlobay requested a review from dajmcdon September 6, 2023 01:49
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.

  1. One structural issue which we can discuss.
  2. A few minor style issues.
  3. There's a stray file R/add_model.Rd to remove.

@rachlobay rachlobay requested a review from dajmcdon September 21, 2023 00:20
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.

I added a number of comments. The ideas are good but a few major changes:

  • Add a tidy() method for frosting objects.
  • Allow the adjust methods to handle names as well as numbers.
  • Create a short vignette that illustrates how these can be used. (including the tidy() methods for examining the recipe/frosting).
  • Run styler so that the code formatting matches the style on main.

@rachlobay rachlobay requested a review from dajmcdon October 25, 2023 18:22
@rachlobay rachlobay merged commit 61a4076 into main Oct 25, 2023
@rachlobay rachlobay deleted the 73-update-layer branch October 25, 2023 18:41
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.

update_layer()
2 participants