Merge pull request #724 from mlr-org/fix_tunethreshold

Fix tunethreshold

Author: mb706

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: mlr3pipelines
 Title: Preprocessing Operators and Pipelines for 'mlr3'
-Version: 0.5.0-9000
+Version: 0.5.0-1
 Authors@R:
     c(person(given = "Martin",
              family = "Binder",

NEWS.md

@@ -1,4 +1,6 @@
-# mlr3pipelines 0.5.0-9000
+# mlr3pipelines 0.5.0-1
+
+* Bugfix: `PipeOpTuneThreshold` was not overloading the correct `.train` and `.predict` functions.
 
 # mlr3pipelines 0.5.0
 

R/PipeOp.R

@@ -364,9 +364,7 @@ PipeOp = R6Class("PipeOp",
     },
     predict_type = function(val) {
       if (!missing(val)) {
-        if (!identical(val, private$.learner)) {
-          stop("$predict_type is read-only.")
-        }
+        stop("$predict_type is read-only.")
       }
       return(NULL)
     },

R/PipeOpLearnerCV.R

@@ -161,6 +161,13 @@ PipeOpLearnerCV = R6Class("PipeOpLearnerCV",
       } else {
         multiplicity_recurse(self$state, clone_with_state, learner = private$.learner)
       }
+    },
+    predict_type = function(val) {
+      if (!missing(val)) {
+        assert_subset(val, names(mlr_reflections$learner_predict_types[[private$.learner$task_type]]))
+        private$.learner$predict_type = val
+      }
+      private$.learner$predict_type
     }
   ),
   private = list(

R/PipeOpTuneThreshold.R

@@ -87,8 +87,13 @@ PipeOpTuneThreshold = R6Class("PipeOpTuneThreshold",
         output = data.table(name = "output", train = "NULL", predict = "Prediction"),
         tags = "target transform"
       )
-    },
-    train = function(input) {
+    }
+  ),
+  active = list(
+    predict_type = function() "response"  # we are predict type "response" for now, so we don't break things. See discussion in #712
+  ),
+  private = list(
+    .train = function(input) {
       if(!all(input[[1]]$feature_types$type == "numeric")) {
         stop("PipeOpTuneThreshold requires predicted probabilities! Set learner predict_type to 'prob'")
       }
@@ -97,13 +102,11 @@ PipeOpTuneThreshold = R6Class("PipeOpTuneThreshold",
       self$state = list("threshold" = th)
       return(list(NULL))
     },
-    predict = function(input) {
+    .predict = function(input) {
       pred = private$.task_to_prediction(input[[1]])
       pred$set_threshold(self$state$threshold)
       return(list(pred))
-    }
-  ),
-  private = list(
+    },
     .objfun = function(xs, pred, measure) {
       lvls = colnames(pred$prob)
       res = pred$set_threshold(unlist(xs))$score(measure)

tests/testthat/test_pipeop_learnercv.R

@@ -98,3 +98,25 @@ test_that("PipeOpLearnerCV - model active binding to state", {
   expect_null(po$learner$state)
   expect_equal(po$learner_model$state, po$state)
 })
+
+test_that("predict_type", {
+  expect_equal(po("learner_cv", lrn("classif.rpart", predict_type = "response"))$predict_type, "response")
+  expect_equal(po("learner_cv", lrn("classif.rpart", predict_type = "prob"))$predict_type, "prob")
+
+  lcv <- po("learner_cv", lrn("classif.rpart", predict_type = "prob"))
+
+  lcv$predict_type = "response"
+  expect_equal(lcv$predict_type, "response")
+  expect_equal(lcv$learner$predict_type, "response")
+
+  expect_equal(lcv$train(list(tsk("iris")))[[1]]$feature_names, "classif.rpart.response")
+
+  lcv$predict_type = "prob"
+
+  expect_equal(lcv$predict_type, "prob")
+  expect_equal(lcv$learner$predict_type, "prob")
+
+  expect_subset(c("classif.rpart.prob.virginica", "classif.rpart.prob.setosa", "classif.rpart.prob.versicolor"),
+    lcv$train(list(tsk("iris")))[[1]]$feature_names)
+
+})

tests/testthat/test_pipeop_tunethreshold.R

@@ -34,3 +34,24 @@ test_that("threshold works for binary", {
     po("tunethreshold")
   expect_error(po_cv$train(t), "prob")
 })
+
+test_that("tunethreshold graph works", {
+
+  graph = po("learner_cv", lrn("classif.rpart", predict_type = "prob")) %>>% po("tunethreshold")
+
+  out = graph$train(tsk("pima"))
+
+  expect_null(out$tunethreshold.output)
+
+  out = graph$predict(tsk("pima"))
+
+  expect_prediction(out$tunethreshold.output)
+
+  glrn = as_learner(graph)
+
+  glrn$train(tsk("pima"))
+
+  expect_prediction(glrn$predict(tsk("pima")))
+
+
+})