fix(extract_inner_fselect_results): copy the result before adding columns (#172)

Author: be-marc

NEWS.md

@@ -1,6 +1,7 @@
 # mlr3fselect (development version)
 
 * fix: `ArchiveAsyncFSelect` pushed results with the removed `rush::Rush$push_results()` method.
+* fix: `extract_inner_fselect_results()` added the `iteration` and `fselect_instance` columns to the result of the inner `FSelectInstance` by reference, which created a circular reference between the instance and its own result (#172).
 * fix: `fs("rfe")` and `fs("rfecv")` failed with an internal `data.table` error when `store_benchmark_result = FALSE` was set because the importance scores were read from the benchmark result of the archive (#169).
 * fix: `fs("rfecv", recursive = FALSE)` failed with an internal `data.table` error because the importance scores of all resampling iterations were written to a single archive row (#168).
 * fix: `fs("rfecv")` ignored the direction of the measure and selected the feature set size with the worst mean performance for minimizing measures such as `msr("classif.ce")` or `msr("regr.mse")`.

R/extract_inner_fselect_results.R

@@ -63,7 +63,8 @@ extract_inner_fselect_results.ResampleResult = function(x, fselect_instance = FA
     return(data.table())
   }
   tab = imap_dtr(rr$learners, function(learner, i) {
-    data = setalloccol(learner$fselect_result)
+    # copy the result, otherwise the columns below are added to the result of the instance
+    data = copy(learner$fselect_result)
     set(data, j = "iteration", value = i)
     if (fselect_instance) {
       set(data, j = "fselect_instance", value = list(learner$fselect_instance))

tests/testthat/test_extract_inner_fselect_result.R

@@ -377,3 +377,27 @@ test_that("extract_inner_fselect_results function works with benchmark and retur
   )
   expect_equal(unique(ibmr$experiment), c(1, 2))
 })
+
+test_that("extract_inner_fselect_results does not modify the fselect result", {
+  rr = fselect_nested(
+    fs("random_search"),
+    tsk("iris"),
+    lrn("classif.rpart"),
+    rsmp("holdout"),
+    rsmp("cv", folds = 2),
+    msr("classif.ce"),
+    term_evals = 4
+  )
+
+  columns = names(rr$learners[[1]]$fselect_result)
+
+  extract_inner_fselect_results(rr)
+  expect_named(rr$learners[[1]]$fselect_result, columns)
+
+  extract_inner_fselect_results(rr, fselect_instance = TRUE)
+  expect_named(rr$learners[[1]]$fselect_result, columns)
+
+  # the instance is not added again when it is not requested
+  irr = extract_inner_fselect_results(rr)
+  expect_names(names(irr), disjunct.from = "fselect_instance")
+})