Use new condition system for errors and warnings #1398
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| # Replace this with x$format() when the issue is solved. | ||
| format_angle_brackets = function(x) { | ||
| sprintf("<<%s:%s>>", class(x)[1L], x$id) | ||
| } |
There was a problem hiding this comment.
Bug: Double angle brackets in error messages
The format_angle_brackets function produces double angle brackets <<ClassName:id>> instead of single angle brackets <ClassName:id>. This breaks existing test expectations and changes user-facing error message formats. The test at line 252 expects <LearnerRegrRpart:regr.rpart> but will receive <<LearnerRegrRpart:regr.rpart>>. While the function is intended as a workaround for a cli issue with angle brackets, it inadvertently changes the output format in a breaking way.
Additional Locations (2)
There was a problem hiding this comment.
Bug: Incomplete refactoring: stopf not converted to error function
The refactoring to replace stopf with the new condition system functions is incomplete. Three instances of stopf remain unchanged while surrounding code was converted to use error_input and other new error functions. This creates inconsistency in error handling across the codebase and means these errors won't benefit from the new condition system that handles angle brackets correctly.
R/Measure.R#L341-L342
Lines 341 to 342 in 7b5c2b6
R/PredictionDataClassif.R#L112-L113
mlr3/R/PredictionDataClassif.R
Lines 112 to 113 in 7b5c2b6
R/PredictionDataRegr.R#L109-L110
Lines 109 to 110 in 7b5c2b6
There was a problem hiding this comment.
Bug: Inconsistent error function usage in Measure class
The PR aims to replace all stopf() calls with the new error system, but several instances were missed. In Measure$obs_loss() at line 342, stopf() is still used for the same error message that was converted to error_input() in Measure$score() at line 267. Similarly, in PredictionDataClassif.R and PredictionDataRegr.R, the error about extra data still uses stopf() while the surrounding errors about weights and predict types were converted to error_input(). These inconsistencies suggest incomplete migration to the new condition system.
R/Measure.R#L341-L342
Lines 341 to 342 in a073ba9
R/PredictionDataClassif.R#L112-L113
mlr3/R/PredictionDataClassif.R
Lines 112 to 113 in a073ba9
R/PredictionDataRegr.R#L109-L110
Lines 109 to 110 in a073ba9
|
|
||
| if (roll("message_train")) { | ||
| message("Message from classif.debug->train()") | ||
| message("Message from regr.debug->train()") |
There was a problem hiding this comment.
Bug: Incorrect learner name in debug message
The training message for regr.debug incorrectly references classif.debug instead of regr.debug. The message says "Message from classif.debug->train()" but this is the regression debug learner, so it should say "Message from regr.debug->train()". The predict message at line 134 correctly uses "regr.debug", showing this is an inconsistency.
R/LearnerRegr.R
Outdated
|
|
||
| if ("quantiles" %nin% self$predict_types) { | ||
| stopf("Learner does not support predicting quantiles") | ||
| error_learner_predict("Learner does not support predicting quantiles") |
There was a problem hiding this comment.
Bug: Wrong error type for quantile configuration
The errors when setting quantiles and quantile_response fields use error_learner_predict but should use error_config. These errors occur during configuration when a user tries to set quantile-related fields on a learner that doesn't support quantile prediction, not during actual prediction. Using error_learner_predict incorrectly categorizes this as a prediction failure rather than a configuration error.
error_*(msg, ...)andwarning_*(msg, ...)functions currently cannot handle messages where angle brackets are used, i.e.<*>. This is due to a bug incli(see r-lib/cli#789) that will hopefully be fixed in the future.Note
Migrate the codebase to a new structured error/warning system, updating messages, docs, and tests accordingly.
stopf/warningfwith structured condition helpers (error_input,error_config,error_mlr3,error_learner_*,warning_config,warning_input,warning_mlr3) across learners, tasks, measures, resampling, predictions, data backends, and utils.format_angle_brackets()to safely format objects in messages (workaround for cli angle-bracket bug).Written by Cursor Bugbot for commit 2835ef2. This will update automatically on new commits. Configure here.