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.

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

Implement update_role_requirements() #1011

Merged

Conversation

DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented Jun 23, 2022

Closes #1009

Joint PR with tidymodels/hardhat#207

This replaces check_new_data_columns() as a more flexible way of specifying what is or is not required at bake time. It has smart defaults and backwards compatibility with old recipes built in.

After the hardhat updates:

  • Update (or remove?) bake_dependent_roles.Rmd
  • Update spatialsign.R example that uses bake_dependent_roles
  • Update test_colcheck.R test that uses bake_dependent_roles
  • Depend on dev hardhat, not the PR. Do this after merging the hardhat PR.
  • After merging, go back and update hardhat's workflow file R-CMD-check-dev-recipes.yaml to install the main branch of recipes

@DavisVaughan
Copy link
Member Author

DavisVaughan commented Jun 27, 2022

It actually might make more sense for recipes:::get_bake_role_requirements(rec) to return logical() if you haven’t touched anything with update_role_requirements(). Then the algorithm to compute the “full” requirements would be:

  • Start with default_bake_role_requirements()
  • Add non-standard roles in the recipe and set them to TRUE
  • Override with any user defined requirements found in get_bake_role_requirements()

I think that would be less confusing. It would be clearer that get_bake_role_requirements() is a direct reflection of what you’ve modified in update_role_requirements(), and nothing else

Update: Done in 681b2a3

@DavisVaughan DavisVaughan force-pushed the feature/update-role-requirements branch from fabd05c to 681b2a3 Compare June 28, 2022 21:36
@DavisVaughan DavisVaughan force-pushed the feature/update-role-requirements branch 4 times, most recently from ab68b01 to 681b2a3 Compare June 29, 2022 14:56
@DavisVaughan DavisVaughan merged commit 049bbe9 into tidymodels:main Jun 29, 2022
@DavisVaughan DavisVaughan deleted the feature/update-role-requirements branch June 29, 2022 17:33
@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant