rsession get_running_time() method

[djnavarro]

Hi Gábor!

I think there is a problem with the get_running_time() method for rsession objects. Whenever one of the two uptime values (either the total or current) exists, rs_get_running_time calculates a difftime, but when the session has finished the total uptime is specified by as.POSIXct(NA) leading to this error:

rs <- callr::r_session$new()
rs$get_running_time()
#> Error in as.POSIXct.default(e): do not know how to convert 'e' to class "POSIXct"

^{Created on 2022-12-17 with reprex v2.0.2}

I'm running the current dev build 0a099b2

Relevant code snippet:

callr/R/r-session.R

Lines 539 to 544 in 0a099b2

	rs_get_running_time <- function(self, private) {
	now <- Sys.time()
	finished <- private$state == "finished"
	c(total = if (finished) now - private$started_at else as.POSIXct(NA),
	current = now - private$fun_started_at)
	}

If -- and this is always a big if 😁 -- I've understood the intended role of this method, I think it's an easy fix. The edits in https://github.com/djnavarro/callr/tree/hotfix-get-running-time modify rs_get_running_time to alwasy return a pair of difftimes, never POSIXct objects. The first value is NA if the session is finished, the second value is NA if the session is finished or idle:

rs_get_running_time <- function(self, private) {
  now <- Sys.time()
  finished <- private$state == "finished"
  idle <- private$state == "idle"
  no_uptime <- as.difftime(NA_real_, units = "secs")
  c(total = if (finished) no_uptime else now - private$started_at,
    current = if (finished | idle) no_uptime else now - private$fun_started_at)
}

I haven't written any tests but I did check that it seems to do something sensible: returns c(NA, NA) for a finished session, uptime for the current call reverts to NA as soon as session$read() is called, and uptime for any subsequent calls is indeed measured from its own start time not the time at which any preceding calls were started.

Happy to add a couple of tests and send a PR if that's at all helpful 😄

[gaborcsardi]

Thanks! This is the docs:

    #' Returns the elapsed time since the R process has started, and the
    #' elapsed time since the current computation has started. The latter
    #' is `NA` if there is no active computation.
    #' @return Named vector of `POSIXct` objects. The names are `"total"`
    #'   and `"current"`.

So I guess the first should never be NA and the second is NA if nothing is running. Yeah, it should return a difftime object. A PR is most welcome! Thanks again!