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

Support empty selections in all recipe steps #813

Merged
merged 95 commits into from
Sep 28, 2021
Merged

Support empty selections in all recipe steps #813

merged 95 commits into from
Sep 28, 2021

Conversation

DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented Sep 20, 2021

This PR adds empty selection support to all recipe steps.


We thought that the best way to do this was to return early from each step, returning an alternate step_empty_selection() step, but this was problematic for a few reasons:

  • We lose the column structure when you tidy() that step in the recipe, because the empty selection step doesn't know what columns should be there
  • If you prep a recipe, then reprep it with fresh = TRUE, things got a little complicated because ideally you'd reprep with the original step, rather than the empty selection step we inserted
  • It isn't type stable (minor issue)
  • It still feels like a bit of a hack

SO, I've gone through all of the steps, ensuring that each one supports empty selection natively. Most required minimal changes, which is great. A few notes:

  • I've replaced all usage of ellipse_check() with enquos(), since now empty selections are okay. This function is exported, so I also mentioned that it is sort of deprecated now in favor of supporting empty selections directly (but I didn't officially deprecate it).
  • Each step now has 3 tests associated with it related to empty selections. One tests prep() / bake(), one tests the print method, and one tests the tidy() method before and after prepping.
  • Going through all of this fixed a number of small edge case bugs that weren't worth a NEWS bullet. In particular, our tidy methods often contained columns that were named vectors. These have all now been un-named.

But its actually defunct so no tests are needed
@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 Oct 13, 2021
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.

2 participants