Skip to content

New argument call for stopf() and warningf()#186

Open
advieser wants to merge 7 commits into
mainfrom
stopf_call
Open

New argument call for stopf() and warningf()#186
advieser wants to merge 7 commits into
mainfrom
stopf_call

Conversation

@advieser

@advieser advieser commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

We wanted this here in mlr3pipelines.

@advieser

Copy link
Copy Markdown
Contributor Author

Sadly, since we pass a condition object, stop() ignores all other arguments. From the docs of stop():

If a condition object is supplied it should be the only argument, and further arguments will be ignored, with a warning.

Codex suggest

condition_call = if (call) sys.call(-1L) else NULL
condition = structure(
  list(message = as.character(message), call = condition_call),
  class = class
)
stop(condition)

@advieser advieser changed the title Expose argument call. in stopf() and warningf() New argument call for stopf() and warningf() Jun 10, 2026
@advieser

Copy link
Copy Markdown
Contributor Author

Encapsulation tests fail because previously we had

> tryCatch(mlr3misc::stopf("c"), error = identity) |> str()
List of 1
 $ message: chr "c"
 - attr(*, "class")= chr [1:3] "Mlr3Error" "error" "condition"

which now is:

> tryCatch(mlr3misc::stopf("c"), error = identity) |> str()
List of 2
 $ message: chr "c"
 $ call   : language doTryCatch(return(expr), name, parentenv, handler)
 - attr(*, "class")= chr [1:3] "Mlr3Error" "error" "condition"

However, compare this with normal stop() behavior:

> tryCatch(stop("c"), error = identity) |> str()
List of 2
 $ message: chr "c"
 $ call   : language doTryCatch(return(expr), name, parentenv, handler)
 - attr(*, "class")= chr [1:3] "simpleError" "error" "condition"

Recommendation: Update encapsulation tests and leave new implementation as is.

@advieser

advieser commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

With this change, stopf would show the call by default. This is closer to base's stop(). However, previously, we never showed the call with stopf().

Base behavior:

f <- function(msg, call) stop(msg, call. = call)
f("a", TRUE)
# Error in f("a", TRUE) : a
f("a", FALSE)
# Error: a

Old behavior:

f <- function(msg) stopf(msg)
f("a")
# Error: a

New behavior:

f <- function(msg, call) stopf(msg, call = call)
f("a", TRUE)
# Error in f("a", TRUE) : a
f("a", FALSE)
# Error: a

You can also see this for R6 objects. Example code:

en = new.env(parent = asNamespace("mlr3misc"))

Test = R6::R6Class(
  "Test",
  parent_env = en,
  public = list(
    base = function() stop("base"),
    misc = function() stopf("misc")
  )
)

capture_calls = function(object) {
  list(
    base = conditionCall(tryCatch(object$base(), error = identity)),
    misc = conditionCall(tryCatch(object$misc(), error = identity))
  )
}

capture_calls(Test$new())

leanify_r6(Test, en)

capture_calls(Test$new())

Old stopf() output:

$base
object$base()

$misc
NULL

$base
.__Test__base(self = self, private = private, super = super)

$misc
NULL

New stopf() output:

$base
object$base()

$misc
object$misc()

$base
.__Test__base(self = self, private = private, super = super)

$misc
.__Test__misc(self = self, private = private, super = super)

@mb706

mb706 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

@sebffischer you removed showing the 'call' in stopf() etc. here, was this deliberate? What is our reason here?

@advieser

Copy link
Copy Markdown
Contributor Author

This seems to be a remain from this change, which got rolled back with this change.

For leanified R6 classes, the call included in the condition is the method call and not the call into the leanified method.

@sebffischer

Copy link
Copy Markdown
Member
  1. The leanification was an issue that I had solved earlier when adding the call to our condition object (in the initial implementation in mlr3misc we used simpleError.
  2. I also thought initially that showing the call was worthwhile, that's why I added it. However, in practice this then meant that many error messages showed internal helpers as the call, which I thought was more confusing to end users than not having a call at all. I believe we have also discussed this at a workshop when we introduced the error classes.
  3. I guess with the heavy use of LLMs, giving them more information about which function caused the error (i.e. the call) makes more sense as before as they don't mind looking at the mlr3 source code in comparison to our human users I guess, so maybe we should add the call by default again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants