-
Notifications
You must be signed in to change notification settings - Fork 48
Revisit logging in tune_bayes_workflow() #1156
Copy link
Copy link
Open
Labels
featurea feature request or enhancementa feature request or enhancementupkeepmaintenance, infrastructure, and similarmaintenance, infrastructure, and similar
Description
tune_bayes_workflow() contains two code comments about logging, one being an explicit TODO
Line 440 in 3544816
| # Maybe remove .catch_and_log() here and do catching inside function |
Lines 481 to 482 in 3544816
| # TODO move some messages here | |
| # check_and_log_flow(control, candidates) |
check_and_log_flow() is currently unused. It's defined in R/logging.R but is only exercised by tests — this is its only (former) production call site. The function itself uses eval.parent(parse(text = "break")) to manipulate the caller's control flow, which is fragile.
Lines 449 to 476 in 3544816
| check_and_log_flow <- function(control, results) { | |
| if (!isTRUE(control$verbose_iter)) { | |
| return(invisible(NULL)) | |
| } | |
| if (all(is.na(results$.mean))) { | |
| if (nrow(results) < 2) { | |
| update_printer( | |
| control, | |
| split_labels = NULL, | |
| task = "Halting search", | |
| type = "danger", | |
| catalog = FALSE | |
| ) | |
| eval.parent(parse(text = "break")) | |
| } else { | |
| update_printer( | |
| control, | |
| split_labels = NULL, | |
| task = "Skipping to next iteration", | |
| type = "danger", | |
| catalog = FALSE | |
| ) | |
| eval.parent(parse(text = "next")) | |
| } | |
| } | |
| invisible(NULL) | |
| } |
If check_and_log_flow() is superseded, we should remove it and its tests, and clean up the comment.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
featurea feature request or enhancementa feature request or enhancementupkeepmaintenance, infrastructure, and similarmaintenance, infrastructure, and similar