Skip to content

Add engine specific predictor encodings#51

Merged
DavisVaughan merged 12 commits intomasterfrom
add-parsnip-encoding
Jun 4, 2020
Merged

Add engine specific predictor encodings#51
DavisVaughan merged 12 commits intomasterfrom
add-parsnip-encoding

Conversation

@juliasilge
Copy link
Copy Markdown
Member

@juliasilge juliasilge commented Jun 2, 2020

This PR makes workflows aware of the new engine specific predictor encodings implemented in tidymodels/parsnip#319 and discussed in tidymodels/parsnip#290 (and elsewhere).

The main idea is that instead of adding a blueprint to a preprocessor in a new_action_ function, the blueprint argument is passed on and evaluated later, within .fit_pre() in the new `update_model_encoding() function.

  • If the blueprint has been set by the user, no updating takes place. 🚫
  • For the formula preprocessor, we add the default blueprint and then look up the model encoding and update it. 💃
  • For the recipe preprocessor, we only add the default blueprint (no relevant encodings). ✅

The tests use a ranger model because it is more clear what the "right" values should be, but no ranger function is called within .fit_pre() (only parsnip functions).

In one test, we have:

expect_equal(ncol(result$pre$mold$predictors), 6)

If we fix 🤞 the issue around one-hot encoding vs. indicator values, this should go to 5.

@juliasilge juliasilge requested a review from DavisVaughan June 2, 2020 23:00
@DavisVaughan DavisVaughan merged commit 270b2ec into master Jun 4, 2020
@DavisVaughan DavisVaughan deleted the add-parsnip-encoding branch June 4, 2020 15:41
@DavisVaughan
Copy link
Copy Markdown
Member

Thanks!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 6, 2021

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 Mar 6, 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