/ et al. drop class "hms"

[AshesITR]

Most computations with hms drop the class to difftime causing worse UX.

hms::hms(10.5) / 2.0
#> Time difference of 5.25 secs
`/.hms` <- function(a, b) {
  res <- as.difftime(unclass(a), units = attr(a, "units")) / b
  hms::as_hms(res)
}
hms::hms(10.5) / 2.0
#> 00:00:05.25
sum(hms::hms(c(10L, 10L)))
#> Time difference of 20 secs

^{Created on 2023-08-18 with reprex v2.0.2}

[AshesITR]

Here is a more general fix, covering base operations and summaries (like sum, min, max, range), and mean()
I can work on a PR if the change is desired.

Ops.hms <- function(e1, e2) {
  res <- NextMethod(.Generic)
  if (inherits(res, "difftime")) {
    hms::as_hms(res)
  } else {
    res
  }
}

Summary.hms <- function(..., na.rm) {
  res <- NextMethod(.Generic)
  hms::as_hms(res)
}

mean.hms <- function(x, ...) {
  hms::as_hms(NextMethod(.Generic))
}

[krlmlr]

Thanks. Is this related to #18?

[AshesITR]

Trying to understand the neccessary test cases:
Relevant classes appear to be all of: Date, POSIXct and POSIXlt (possibly with timezones and times around DST), difftime (base), hms.

Once with hms on the LHS and once with hms on the RHS (with expect_error() where appropriate).

Is that assessment correct?

[krlmlr]

Multiplying with and dividing by numeric or integer? Otherwise, this looks fairly complete.

[olsgaard]

I'm having a similar problem with max(my_hms_time) returning a difftime instead of a hms, e.g.:

> max(as_hms(10))
Time difference of 10 secs
> class(max(as_hms(10)))
[1] "difftime"
> class(as_hms(10))
[1] "hms"      "difftime"

Would #120 fix this and what would it take to review it again?

[krlmlr]

In #120, we need the existing tests to work. Could require changes in base R. I retriggered the test runs.