Skip to content

Commit ce25460

Browse files
authored
refactor: raise structured errors instead of bare stop() (#198)
* refactor: raise structured errors instead of bare stop() `CLAUDE.md` mandates `cli` for errors, but eleven places raised a plain `simpleError`. They use the `mlr3misc` condition constructors now, which format with `cli` and carry an `Mlr3Error` class that callers can catch. * Update FSelector.R
1 parent d38236c commit ce25460

7 files changed

Lines changed: 22 additions & 12 deletions

R/EnsembleFSResult.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ EnsembleFSResult = R6Class(
214214

215215
# check if `inner_measure` is an `mlr3::Measure`
216216
if (which == "inner" && is.null(private$.inner_measure)) {
217-
stop("No inner_measure was defined during initialization")
217+
error_input("No `inner_measure` was defined during initialization.")
218218
}
219219

220220
private$.active_measure = which

R/FSelector.R

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ FSelector = R6Class(
9696
#' Set of control parameters.
9797
param_set = function(rhs) {
9898
if (!missing(rhs) && !identical(rhs, private$.param_set)) {
99-
stop("$param_set is read-only.")
99+
error_input("`$param_set` is read-only.")
100100
}
101101
private$.param_set
102102
},
@@ -106,7 +106,7 @@ FSelector = R6Class(
106106
#' Must be a subset of [`mlr_reflections$fselect_properties`][mlr3::mlr_reflections].
107107
properties = function(rhs) {
108108
if (!missing(rhs) && !identical(rhs, private$.properties)) {
109-
stop("$properties is read-only.")
109+
error_input("`$properties` is read-only.")
110110
}
111111
private$.properties
112112
},
@@ -116,7 +116,7 @@ FSelector = R6Class(
116116
#' Note that these packages will be loaded via [requireNamespace()], and are not attached.
117117
packages = function(rhs) {
118118
if (!missing(rhs) && !identical(rhs, private$.packages)) {
119-
stop("$packages is read-only.")
119+
error_input("`$packages` is read-only.")
120120
}
121121
private$.packages
122122
},
@@ -126,7 +126,7 @@ FSelector = R6Class(
126126
#' Can be used in tables, plot and text output instead of the ID.
127127
label = function(rhs) {
128128
if (!missing(rhs) && !identical(rhs, private$.param_set)) {
129-
stop("$label is read-only.")
129+
error_input("`$label` is read-only.")
130130
}
131131
private$.label
132132
},
@@ -136,14 +136,14 @@ FSelector = R6Class(
136136
#' The referenced help package can be opened via method `$help()`.
137137
man = function(rhs) {
138138
if (!missing(rhs) && !identical(rhs, private$.man)) {
139-
stop("$man is read-only.")
139+
error_input("`$man` is read-only.")
140140
}
141141
private$.man
142142
}
143143
),
144144

145145
private = list(
146-
.optimize = function(inst) stop("abstract"),
146+
.optimize = function(inst) error_bbotk("`.optimize()` is abstract and must be implemented by the subclass."),
147147

148148
.assign_result = function(inst) {
149149
assert_fselect_instance(inst)

R/FSelectorAsyncFromOptimizerAsync.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ FSelectorAsyncFromOptimizerAsync = R6Class(
5353
#' Set of control parameters.
5454
param_set = function(rhs) {
5555
if (!missing(rhs) && !identical(rhs, private$.optimizer$param_set)) {
56-
stop("$param_set is read-only.")
56+
error_input("`$param_set` is read-only.")
5757
}
5858
private$.optimizer$param_set
5959
}

R/FSelectorBatchSequential.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ FSelectorBatchSequential = R6Class(
6666
optimization_path = function(inst, include_uhash = FALSE) {
6767
archive = inst$archive
6868
if (archive$n_batch == 0L) {
69-
stop("No results stored in archive")
69+
error_input("No results stored in the archive.")
7070
}
7171
uhash = if (include_uhash) "uhash" else NULL
7272
res = archive$data[, head(.SD, 1), by = get("batch_nr")]

R/FSelectorBatchShadowVariableSearch.R

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ FSelectorBatchShadowVariableSearch = R6Class(
7878
#' @return [data.table::data.table]
7979
optimization_path = function(inst) {
8080
if (inst$archive$n_batch == 0L) {
81-
stop("No results stored in archive")
81+
error_input("No results stored in the archive.")
8282
}
8383

8484
# we have to use the best method to get the same tie breaking as in the optimize method
@@ -128,7 +128,7 @@ FSelectorBatchShadowVariableSearch = R6Class(
128128
if (any(as.logical(res[, shadow_variables, with = FALSE]))) {
129129
# stop if the first selected feature is a shadow variable
130130
if (archive$n_batch == 1) {
131-
stop("The first selected feature is a shadow variable.")
131+
error_mlr3("The first selected feature is a shadow variable.")
132132
}
133133

134134
# remove last batch with selected shadow variable from archive

tests/testthat/test_embedded_ensemble_fselect.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ test_that("embedded efs works", {
5252
)
5353

5454
# cannot change to use inner_measure
55-
expect_error(efsr$set_active_measure(which = "inner"), "No inner_measure was defined")
55+
expect_error(efsr$set_active_measure(which = "inner"), "No `inner_measure` was defined")
5656
# changing to "outer" leaves us with the same measure
5757
efsr$set_active_measure(which = "outer")
5858
expect_equal(efsr$measure$id, "classif.ce") # classification error

tests/testthat/test_mlr_fselectors.R

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@ test_that("as.data.table objects parameter", {
1919
expect_list(tab$object, "FSelector", any.missing = FALSE)
2020
})
2121

22+
test_that("read-only bindings of a fselector raise a structured error", {
23+
fselector = fs("random_search")
24+
25+
expect_error(fselector$param_set <- ps(), class = "Mlr3ErrorInput")
26+
expect_error(fselector$properties <- "single-crit", class = "Mlr3ErrorInput")
27+
expect_error(fselector$packages <- "mlr3", class = "Mlr3ErrorInput")
28+
expect_error(fselector$label <- "other", class = "Mlr3ErrorInput")
29+
expect_error(fselector$man <- "other", class = "Mlr3ErrorInput")
30+
})
31+
2232
test_that("reloading the package does not duplicate reflections", {
2333
bbotk_reflections = utils::getFromNamespace("bbotk_reflections", ns = "bbotk")
2434
mlr_reflections = utils::getFromNamespace("mlr_reflections", ns = "mlr3")

0 commit comments

Comments
 (0)