hms class dropped when doing arithmetic on hms objects

[jimhester]

late_night <- hms::hms(seconds = 22 * 3600 + 20 * 60)
str(late_night + 5)
#> Class 'difftime'  atomic [1:1] 80405
#>   ..- attr(*, "units")= chr "secs"

I think you just need to define Ops.hms that calls Ops.difftime and preserves the hms class.

[jimhester]

The following works for the test case, but generates a warning and incorrect results when doing arithmetic with Date objects.

Ops.hms <- function(e1, e2) {
  res <- NextMethod("Ops")
  mostattributes(res) <- attributes(e1)
  res
}

late_night <- hms::hms(seconds = 22 * 3600 + 20 * 60)
str(late_night + 5)
#> Classes 'hms', 'difftime'  atomic [1:1] 80405
#>   ..- attr(*, "units")= chr "secs"

as.Date("2016-03-31") + hms(minutes = 1)
#> Warning: Incompatible methods ("+.Date", "Ops.hms") for "+"
#> [1] "2016-05-30"

[krlmlr]

It's an annoyance, but I'm not sure it can be fixed easily. If I define Ops.hms(), many existing tests fail.

[krlmlr]

I'm giving up. R has some special handling for the difftime class in its internals, it seems impossible to implement this without breaking cases where we add difftime or hms to a date or date-time object:

https://github.com/wch/r-source/blob/a46559e8f728317da979be60e401899ae60086b2/src/main/eval.c#L3406-L3419

[github-actions[bot]]

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

[krlmlr]

Finally found a way that seems to work:

#' @export
Ops.hms <- function(e1, e2) {
  out <- ...
  if (inherits(out, "difftime")) {
    out <- vec_cast(out, new_hms())
  }
  out
}

# Registered on .onLoad() to avoid warning when loading the package
`+.Date` <- Ops.hms

If I override +.Date, the warning disappears when adding hms and date objects, I can control the implementation. The delayed registration gets rid of the warning when loading the package.

+.Date() does nothing spectacular. Same for -.Date(), +.POSIXt and -.POSIXt.

With this I think we can finally get full arith and generic support.

Related: #97.

[krlmlr]

Prior to implementing, need to add tests for all combinations of adding and subtracting dates, datetimes and difftimes. These must work unchanged after the implementation.