Skip to content

Commit

Permalink
require optional arguments to be named (#105)
Browse files Browse the repository at this point in the history
* address issues from tidymodels/tune#863

* move dots in `collect_*()`

* move dots in `show_best.tune_race()`

note that this adds a `call` argument for compatibility with the `tune_results` method--otherwise, tune will add one when calling `show_best()` inside of `select_best()` and trigger a nonempty dots error

* note change in NEWS

* update Remotes ref

* pass `type` by name

preserves the previous behavior of respecting `type`; this is already tested in extratests

* update tune ref [no ci]
  • Loading branch information
simonpcouch authored Mar 1, 2024
1 parent 7bbb18f commit c127f2e
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 28 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ URL: https://github.com/tidymodels/finetune,
BugReports: https://github.com/tidymodels/finetune/issues
Depends:
R (>= 3.5),
tune (>= 1.1.2.9011)
tune (>= 1.1.2.9020)
Imports:
cli,
dials (>= 0.1.0),
Expand Down
5 changes: 2 additions & 3 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@

* Enabling the `verbose_elim` control option for `tune_race_anova()` will now additionally introduce a message confirming that the function is evaluating against the burn-in resamples.

* Updates based on the new version of tune, primarily for survival analysis models.
* Updates based on the new version of tune, primarily for survival analysis models (#104).

## Breaking Change

* `show_best.tune_race()` gains an `eval_time` argument for censored regression models. This breaks passing `n` by position (#104).

* Ellipses (...) are now used consistently in the package to require optional arguments to be named. `collect_predictions()`, `collect_metrics()` and `show_best()` methods previously had ellipses at the end of the function signature that have been moved to follow the last argument without a default value. Optional arguments previously passed by position will now error informatively prompting them to be named (#105).

# finetune 1.1.0

Expand Down
22 changes: 15 additions & 7 deletions R/racing_helpers.R
Original file line number Diff line number Diff line change
Expand Up @@ -665,12 +665,13 @@ randomize_resamples <- function(x) {
#' @name collect_predictions
collect_predictions.tune_race <-
function(x,
...,
summarize = FALSE,
parameters = NULL,
all_configs = FALSE,
...) {
all_configs = FALSE) {
rlang::check_dots_empty()
x <- dplyr::select(x, -.order)
res <- NextMethod(summarize = summarize, parameters = parameters)
res <- collect_predictions(x, summarize = summarize, parameters = parameters)
if (!all_configs) {
final_configs <- subset_finished_race(x)
res <- dplyr::inner_join(res, final_configs, by = ".config")
Expand All @@ -681,10 +682,11 @@ collect_predictions.tune_race <-
#' @inheritParams tune::collect_metrics
#' @export
#' @rdname collect_predictions
collect_metrics.tune_race <- function(x, summarize = TRUE, all_configs = FALSE, ...) {
collect_metrics.tune_race <- function(x, ..., summarize = TRUE, type = c("long", "wide"), all_configs = FALSE) {
rlang::check_dots_empty()
x <- dplyr::select(x, -.order)
final_configs <- subset_finished_race(x)
res <- NextMethod(summarize = summarize, ...)
res <- collect_metrics(x, summarize = summarize, type = type)
if (!all_configs) {
final_configs <- subset_finished_race(x)
res <- dplyr::inner_join(res, final_configs, by = ".config")
Expand All @@ -704,7 +706,13 @@ collect_metrics.tune_race <- function(x, summarize = TRUE, all_configs = FALSE,
#' resampled). Comparing performance metrics for configurations averaged with
#' different resamples is likely to lead to inappropriate results.
#' @export
show_best.tune_race <- function(x, metric = NULL, eval_time = NULL, n = 5, ...) {
show_best.tune_race <- function(x,
...,
metric = NULL,
eval_time = NULL,
n = 5,
call = rlang::current_env()) {
rlang::check_dots_empty()
if (!is.null(metric)) {
# What was used to judge the race and how are they being sorted now?
metrics <- tune::.get_tune_metrics(x)
Expand All @@ -722,7 +730,7 @@ show_best.tune_race <- function(x, metric = NULL, eval_time = NULL, n = 5, ...)
x <- dplyr::select(x, -.order)
final_configs <- subset_finished_race(x)

res <- NextMethod(metric = metric, eval_time = eval_time, n = Inf, ...)
res <- NextMethod(metric = metric, eval_time = eval_time, n = Inf, call = call)
res$.ranked <- 1:nrow(res)
res <- dplyr::inner_join(res, final_configs, by = ".config")
res$.ranked <- NULL
Expand Down
4 changes: 2 additions & 2 deletions R/tune_race_anova.R
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ tune_race_anova_workflow <-

opt_metric_time <- tune::first_eval_time(
metrics,
opt_metric_name,
eval_time,
metric = opt_metric_name,
eval_time = eval_time,
call = call
)

Expand Down
4 changes: 2 additions & 2 deletions R/tune_race_win_loss.R
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ tune_race_win_loss_workflow <-

opt_metric_time <- tune::first_eval_time(
metrics,
opt_metric_name,
eval_time,
metric = opt_metric_name,
eval_time = eval_time,
call = call
)

Expand Down
4 changes: 2 additions & 2 deletions R/tune_sim_anneal.R
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,8 @@ tune_sim_anneal_workflow <-
eval_time <- tune::check_eval_time_arg(eval_time, metrics, call = call)
opt_metric_time <- tune::first_eval_time(
metrics,
opt_metric_name,
eval_time,
metric = opt_metric_name,
eval_time = eval_time,
call = call
)

Expand Down
20 changes: 16 additions & 4 deletions man/collect_predictions.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 16 additions & 7 deletions man/show_best.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c127f2e

Please sign in to comment.