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

breaking change in upcoming tune release #55

Open
simonpcouch opened this issue May 31, 2024 · 5 comments
Open

breaking change in upcoming tune release #55

simonpcouch opened this issue May 31, 2024 · 5 comments

Comments

@simonpcouch
Copy link
Collaborator

In the tune release following 1.2.1, tune's .catch_and_log(split) argument will be renamed to .catch_and_log(split_labels), and will take the format labels(split) rather than split. agua just passes that argument once here:

agua/R/tune.R

Lines 105 to 111 in 6a742f6

workflow <- tune::.catch_and_log(
.expr = workflows::.fit_pre(workflow, training_frame),
control,
split,
iter_msg_preprocessor,
notes = out_notes
)

...and can pass it's value conditional on tune's package version.

Related to tidymodels/tune#909.

The noted release is probably at least a couple months out, so this can be ignored for now.

@qiushiyan
Copy link
Collaborator

qiushiyan commented Jun 4, 2024

Sounds good. I will update in the next few days.

The noted release is probably at least a couple months out, so this can be ignored for now.

Not sure I understand this, should I just pass labels(split) or do a conditional on tune's version?

@simonpcouch
Copy link
Collaborator Author

The noted release is probably at least a couple months out, so this can be ignored for now.

We won't be sending tune to CRAN for at least a couple months, so this changes requested in this issue aren't needed yet. The more important move, from tidymodels' perspective, is to send your current development version out (#54 has a model for doing so) that supports the current tune version—CRAN tune and CRAN agua have been incompatible since April 18th.

should I just pass labels(split) or do a conditional on tune's version?

Conditional on tune version, either split or labels(split). Current CRAN tune needs split, the next version will need labels(split). I'm glad to put in a PR with the needed changes once you have a new release of agua out if that's appreciated!

@qiushiyan
Copy link
Collaborator

Thanks for your help @simonpcouch , new CRAN agua is released. I will hopefully be less occupied in the following months so I could make the split change once it's needed. I'll let you know if I need anything.

@simonpcouch
Copy link
Collaborator Author

Hey @qiushiyan, I see you've closed this issue but am not seeing the related fix on any branches. I will reopen so that it stays on our radars when we start to think about the next tune release, but feel free to close if this indeed fixed and I've missed where.

@simonpcouch simonpcouch reopened this Jun 7, 2024
@qiushiyan
Copy link
Collaborator

I forgot to push this. It's in 56b4e0a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants