-
Notifications
You must be signed in to change notification settings - Fork 112
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
The bake.step_log()
S3 method breaks legacy recipe objects
#1284
Comments
Hello! This is indeed a bug. And should be fixed. It is kinda hard testing recipes steps for backwards compatibility so sometimes these things fall through the cracks. This is a high priority issue Would you be able to freeze the installed {recipes} versions? Or were the recent update necessary? Your temporary fix would work as well I'm out of office without computer, coming back on 15th. |
Hi @EmilHvitfeldt , In our production environment we intentionally run a few versions behind "live CRAN" and do an annual update, which is when we caught this issue. Waiting for a bug-fix likely won't work since we cannot use a development version (for reproducibility we must use a released CRAN version). So I'll use the workaround-fix adding the temporary names to the object prior to baking, which should allow backward compatibility internally with our code base too. I definitely know how things can slip through the cracks 🤷🏽♂️ Thanks! 🙌🏽 |
- legacy recipes index the `columns` entry of `object` whereas newer recipes index `names(object$columns)` - this breaks backward compatibility - This bugfix prefers the 'new' syntax, but if `names(object)` is `NULL`, reverts to the 'old' syntax - fixes tidymodels#1284
@stufield A new version is on CRAN with your change. Thanks! |
Thank you @topepo ! |
This issue 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. |
The problem
The current (CRAN
v1.0.9
) versions ofbake.step_log()
breaks older (legacy) recipes made prior to (guessing)v1.0.4
, > 1 year ago. When attempting tobake()
a prepped recipe that has alog_step
, thebake()
S3 method used to check for the appropriate columns using thex$columns
entry of the object, however it now looks for its names, i.e.names(x$columns)
, which for older recipes without vector names will beNULL
, and breaks backward compatibility of the object.The line where this happens is: https://github.com/tidymodels/recipes/blob/main/R/log.R#L127
And it appears to have been introduced here: 6155183
Perhaps even worse, the
bake()
step does not fail when names areNULL
, it skips the logging-loop and exits without modifying the any columns, and resulting in incorrect pre-processed data.Questions:
v1.0.9
?The last option is not ideal since in my use case, these recipes (~45) are production level objects that are frozen with their corresponding model objects and are tied to regulatory controlled customer products.
Reprex
I do not have a
reprex
per se, but there is a simple workaround:The text was updated successfully, but these errors were encountered: