r-lib / pak

A fresh approach to package installation
https://pak.r-lib.org
646 stars 57 forks source link

Output infelicities on Google Colab #464

Closed jennybc closed 1 year ago

jennybc commented 1 year ago

I was demonstrating something (else) to @hadley recently on Google Colab and, along the way, I used pak to install a package there. He suggested I record the output here, in case it suggests some possible improvements.

Here are a couple of screenshots. In many places, the messages are duplicated and I think there are also extra linebreaks.

FYI: Colab is a challenging environment (which I am also grapping with re: httr and gargle), in that it is morally interactive, in some sense. There probably is a user there. But base::interactive() reports FALSE.

Screenshot 2023-02-24 at 11 05 44 AM Screenshot 2023-02-24 at 11 07 12 AM Screenshot 2023-02-24 at 11 08 34 AM
hadley commented 1 year ago

I'm mostly concerned that this might be a general problem when using pak inside jupyter.

gaborcsardi commented 1 year ago

The empty lines seem to be a bug in the Jupyter kernel. It appends a \n to messages:

{ message("foo", appendLF = FALSE); message("bar") }

should print

foobar

but on Jupyter it prints

foo
bar

(With the trailing empty line at the end.)

The duplication is a bug in pak. callr re-emits conditions from the subprocess like this:

f <- function() {
  cond <- structure(
    list(message = "foo\n"),
    class = c("callr_message", "message", "condition")
  )
  withRestarts({
    signalCondition(cond)
  }, muffleMessage = function() NULL)
}

and pak wraps this, to write to stdout() instead of stderr():

g <- function() {
  withCallingHandlers(
    callr_message = function(msg) {
      withRestarts({
        signalCondition(msg)
        out <- if (is_interactive() || sink.number() > 0) stdout() else stderr()
        cat(conditionMessage(msg), file = out, sep = "")
      }, muffleMessage = function() NULL)
    },
    f()
  )
}

This lacks a muffleMessage jump, so if you were to catch the messages, you'll get duplicate ones:

❯ evaluate::evaluate(quote(g()))
[[1]]
$src
[1] "g()"

attr(,"class")
[1] "source"

[[2]]
<callr_message: foo
>

[[3]]
<callr_message: foo
>

[[4]]
[1] "NULL\n"

After adding the muffleMessage at the right place:

g <- function() {
  withCallingHandlers(
    callr_message = function(msg) {
      withRestarts({
        signalCondition(msg)
        out <- if (is_interactive() || sink.number() > 0) stdout() else stderr()
        cat(conditionMessage(msg), file = out, sep = "")
      }, muffleMessage = function() NULL)
      invokeRestart("muffleMessage")
    },
    f()
  )
}

we get instead:

❯ evaluate::evaluate(quote(g()))
[[1]]
$src
[1] "g()"

attr(,"class")
[1] "source"

[[2]]
<callr_message: foo
>

[[3]]
[1] "NULL\n"
hadley commented 1 year ago

@gaborcsardi would you mind making sure the trailing newline bug is tracked in the appropriate place in jupyter?

gaborcsardi commented 1 year ago

Seems like by choice: https://github.com/IRkernel/IRkernel/commit/d7f0b08582c66a1b5119b50e3e6aa56a03b06d1b

But I opened an issue nevertheless: https://github.com/IRkernel/IRkernel/issues/732

hadley commented 1 year ago

Thanks!