Skip to content

unexport spec checking helpers #809

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

Merged
merged 1 commit into from
Sep 15, 2022
Merged

unexport spec checking helpers #809

merged 1 commit into from
Sep 15, 2022

Conversation

simonpcouch
Copy link
Contributor

After working through integrating these new helpers in tidymodels, it looks like we don't actually need to use them outside of parsnip. A note from the last planned PR to integrate them, adding a failure on add_model in workflows:

The thought is that we fail early enough to prevent people from running computationally intensive things before running into issues [with missing extensions]. In practice, I think parsnip makes this happen most of the time: if a user is tuning a workflow and that workflow uses an engine that isn't loaded, then all models will fail with the parsnip error and that tuning will only take a second or two.

It is true that if a user is tuning a workflowset, and some needed extensions are loaded while others are not, the specs with loaded extensions will tune fully before notifying the user that the others failed...

@simonpcouch simonpcouch requested a review from hfrick September 15, 2022 19:56
@topepo topepo merged commit 312d191 into main Sep 15, 2022
@topepo topepo deleted the unexport-check-helpers branch September 15, 2022 20:44
@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 Sep 30, 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.

2 participants