Add generic operations for hms

[AshesITR]

Opening this draft for early feedback.

Missinng POSIXct and POSIXlt currently.
One warning condition (difftime - Date) can't be reproduced exactly using the current implementation.
Let me know if that is a problem, because it seems hard to work around - the compatibility warning will be silenced for good by -.Date <- Ops.hms and this constellation is one where the C++ code regarding difftime has no manual disambiguation, firing the warning.

Fixes #119
Fixes #18

[AshesITR]

NTS: Should also check this plays nicely with {lubridate} et al.

[krlmlr]

Thanks. The existing tests currently fail?

[AshesITR]

Yes, the arith-tests regarding POSIX times.

[AshesITR]

NB We produce this message:

Registered S3 methods overwritten by 'hms':
  method   from
  +.Date   base
  +.POSIXt base
  -.Date   base
  -.POSIXt base

[AshesITR]

R 3.6 and R 4.0 seem to behave differently. @krlmlr do you have an idea what was changed from 4.0 to 4.1?
Also, how to proceed?

Create backward-compatible behavior in the old versions (likely by branching in Ops.hms based on getRVersion() < "4.1")?
Looking at the failures, this seems like the right thing to do, because test-arith.R also produces failures on those versions.

[krlmlr]

I don't mind waiting until R 4.0 is phased out, or at least supporting this feature only for that version.

I plan to return to this package some time later in this year and can only offer very superficial advice in the meantime.

[AshesITR]

Wow, what a weird failure.
Not exporting the methods is ignored by R <= 4.0, causing test-arith failures and an incompatibility warning 😤

Incompatible methods ("+.Date", "Ops.hms") for "+"

The incompatibility warning will not be fixable in R < 4.1 because this fix is needed in eval.c:

https://github.com/wch/r-source/blob/61468d9909b8ab59a25172e584e4400427b40a27/src/main/eval.c#L4114-L4140

https://github.com/wch/r-source/blob/97fee142299e5b418cdaf1057eca0e12c250a2b8/src/main/eval.c#L3796-L3804

Seems like I have to add a working implementation for old R versions, too.

[AshesITR]

I'm failing to reproduce the failure on 4.0.4 @ windows.

[krlmlr]

Much better now, only R 4.0 fails. Support for that is dropped in April, I think it's good to start reconsidering this.

[krlmlr]

Thanks! I'd really love to see this in hms, would make a good 2.0.0 .

[krlmlr]

Should this wrap as hms?

[krlmlr]

We can skip_if(getRversion() < "4.1") .

[krlmlr]

The test chunk is too large, can you please split by topic?

[AshesITR]

I'll see if I can put a little bit more time into this. Spare time for pet projects has taken a cut lately 😅

[krlmlr]

Only R 4.0 seems to be failing. Let's merge this when R 4.5 is out.

[krlmlr]

Also want to make sure to have good test coverage for operations combining hms with dates, times etc.: #18 (comment)