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

remove check_factor_vars() and make step_dummy() error for character input #1233

Merged
merged 7 commits into from
Nov 2, 2023

Conversation

EmilHvitfeldt
Copy link
Member

@EmilHvitfeldt EmilHvitfeldt commented Oct 6, 2023

To close #1226

According to the documentation, step_dummy() is able to take character and factor variables.

https://github.com/tidymodels/recipes/blob/main/R/dummy.R#L3-L5

This was not the case, as check_factor_vars() would exclude non-factor variables. This PR fixes that discrepancy by allowing step_dummy() to take in character predictors. Which was only really an issue when strings_as_factors = FALSE

This PR fixes that by letting step_dummy() error on character input. Which only happens when strings_as_factors = FALSE

@EmilHvitfeldt EmilHvitfeldt changed the title remove check_factor_vars() and allow step_dummy() to take character input remove check_factor_vars() and make step_dummy() error for character input Oct 10, 2023
Merge commit 'fec8b9266e32b54bbb40d2c7d2e597b311d4b71d'

#Conflicts:
#	tests/testthat/_snaps/dummy.md
#	tests/testthat/_snaps/misc.md
@EmilHvitfeldt EmilHvitfeldt merged commit a63ad80 into main Nov 2, 2023
9 checks passed
@EmilHvitfeldt EmilHvitfeldt deleted the phase-out-factor-checking-instep_dummy branch November 2, 2023 23:23
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 Nov 17, 2023
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.

phase out recipes::check_factor_vars()
2 participants