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

Update vfold.R #471

Merged
merged 2 commits into from Apr 22, 2024
Merged

Update vfold.R #471

merged 2 commits into from Apr 22, 2024

Conversation

ZWael
Copy link
Contributor

@ZWael ZWael commented Apr 12, 2024

Hello,
this pull request is for bug correction with vfold_splits function
actually the vfold_splits function do not forward the breaks argument to vfold_splits call

Symptoms
the de fault breaks = 4 is used instead

library(tidymodels)
set.seed(123)
d=iris[1:50,]
folds=vfold_cv(d, v = 5, repeats = 2,strata = Petal.Width)
#> Warning: The number of observations in each quantile is below the recommended threshold of 20.
#> • Stratification will use 2 breaks instead.
#> The number of observations in each quantile is below the recommended threshold of 20.
#> • Stratification will use 2 breaks instead.
folds=vfold_cv(d, v = 5, repeats = 2,strata = Petal.Width,
               breaks = 2)
#> Warning: The number of observations in each quantile is below the recommended threshold of 20.
#> • Stratification will use 2 breaks instead.
#> The number of observations in each quantile is below the recommended threshold of 20.
#> • Stratification will use 2 breaks instead.

obviously not forwarding the breaks argument

no error after vfold_splits call modification

Created on 2024-04-12 with reprex v2.1.0

bug correction with vfold_splits function
forward on breaks value from the vfold_cv function to vfold_splits
@ZWael
Copy link
Contributor Author

ZWael commented Apr 17, 2024

Hello @hfrick,
Could you check my bug fix ?

@hfrick
Copy link
Member

hfrick commented Apr 22, 2024

Thanks for the indirect bug report and the PR!

Here's the slightly reworked reprex for my own reference.

With this PR:

library(rsample)

iris_small <- iris[1:50,]

set.seed(123)

# default breaks = 4
folds <- vfold_cv(iris_small, v = 5, repeats = 2, strata = Petal.Width)
#> Warning: The number of observations in each quantile is below the recommended threshold of 20.
#> • Stratification will use 2 breaks instead.
#> The number of observations in each quantile is below the recommended threshold of 20.
#> • Stratification will use 2 breaks instead.

# this should not warn anymore
folds <- vfold_cv(iris_small, v = 5, repeats = 2, strata = Petal.Width, breaks = 2)

Created on 2024-04-22 with reprex v2.1.0

@hfrick hfrick merged commit a976356 into tidymodels:main Apr 22, 2024
12 checks passed
Copy link

github-actions bot commented May 8, 2024

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 May 8, 2024
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.

None yet

2 participants