solve the summary issue by defining 'names<-'; fix issue with 'c'

[lsilvest]

It was enough to define names<- in order for summary.integer64 to work on nanotime.

There was an issue with c, it was using the double value of nanotime instead of its integer64 value. Interestingly, the tests work because nanotime is originally a double in disguise. So RUnit::checkEquals, which ultimately calls all.equal(target, current, msg="", tolerance = .Machine$double.eps^0.5) returns TRUE when the values are close together. So checkEquals(nanotime(1), nanotime(2)) returns TRUE (and this is the case for integer64 too). That's annoying, but I think it's the kind of thing nanotime will have to live with, because at the end of the day, despite its elegance, integer64 is still a hack...

[joshuaulrich]

If you expect nanotime objects to be the same, I suggest you use RUnit::checkIdentical(nanotime(1), nanotime(2)). You usually should not do that with doubles because of floating point arithmetic rounding error, but that should not happen with integer64 operations because all operations should take place as 64-bit integers. I would expect some bug in integer64 if checkIdentical returned FALSE after some operation involving only integer64 objects and/or subclasses.

[eddelbuettel]

It's a good point. We should really compare bitwise.

[lsilvest]

That's all true, but expect users (me first even so I should know better) to get confused by the floating-point semantics of a type that presents itself as discrete. It's true for the underlying type integer64 too; compare:

> checkEquals(1L, 2L)
Error in checkEquals(1L, 2L) : Mean relative difference: 1
> checkEquals(as.integer64(1), as.integer64(2))
[1] TRUE

[eddelbuettel]

But that is due to the sqrt(eps) precision you cited earlier, and "hence known to be wrong".

[lsilvest]

Yes, yes, but it's about what the users expect: RUnit::checkEquals is supposed to be a fairly general and reasonable check for equality in unit tests. If one doesn't know about the special implementation of nanotime one doesn't have any reason to suspect that the use of this function is incorrect with nanotime. (And also because it is correct with integer and POSIXct).

[joshuaulrich]

expect users to get confused by the floating-point semantics of a type that presents itself as discrete

I'm not sure about that, given that so many R users get confused about the float-point semantics of floating point types (i.e. R FAQ 7.31).

If one doesn't know about the special implementation of nanotime one doesn't have any reason to suspect that the use of this function is incorrect with nanotime.

Agreed. My point is mainly about the unit tests, which are for developers much more than users. And developers are responsible for knowing/understanding the edge cases of their software. So that's why I suggested that you use checkIdentical in unit tests on nanotime objects. Otherwise you may get tests that pass when they shouldn't (if checkEquals is used).

I'm not suggesting that your expectation and use of checkEquals was unreasonable... but that the hack/work-around integer64 uses makes using checkEquals in unit tests potentially dangerous.

---

This also suggests that there should be a all.equal.integer64 method, since all.equal is generic.