Support for abs()

[travis-leith]

This is apparently causing a problem for displaying a nanotime column in a tibble.
r-lib/pillar#378

[eddelbuettel]

Can you work out minimally completely verifiable example not involving other packages?

A minimal one would, indeed, that we not support abs(x) for integer64 types:

> min(x)
[1] 2011-12-05T08:30:00+00:00
> abs(x)
Error in abs(x) : non-numeric argument to mathematical function
> sin(x)
Error in sin(x) : non-numeric argument to mathematical function
> 

There are as shown many other ops we do not support. Yet data.table has printed nanotime (and integer64) for many years, many pillar can look it does it?

> d <- data.table(ii = as.integer64(1:3), nn = as.nanotime(Sys.time()) + 0:2)
> d
   ii                                  nn
1:  1 2021-12-17T14:09:09.119302000+00:00
2:  2 2021-12-17T14:09:09.119302001+00:00
3:  3 2021-12-17T14:09:09.119302002+00:00
> print(d, class=TRUE)
      ii                                  nn
   <i64>                          <nanotime>
1:     1 2021-12-17T14:09:09.119302000+00:00
2:     2 2021-12-17T14:09:09.119302001+00:00
3:     3 2021-12-17T14:09:09.119302002+00:00
> 

We can add abs() but are we sure that fixes your pillar issue?

[travis-leith]

library(nanotime)
x <- nanotime('2011-12-05 08:30:00.000', format ="%Y-%m-%d %H:%M:%E9S", tz ="GMT")
abs(x)

results in
Error in abs(x) : non-numeric argument to mathematical function

Not sure it makes sense to support abs here, but it seems pillar_shaft expects it.

[eddelbuettel]

Yep. I edited my initial answer. data.table gets by without it, probably the main reason we never needed it.

[travis-leith]

We can add abs() but are we sure that fixes your pillar issue?

I don't know but maybe @DavisVaughan (not sure how to mention someone not part of this thread?) can answer.
My interest in this is to be able to use nanotime in conjunction with tidyverse/dplyr, and dplyr depends on pillar.

[eddelbuettel]

Again. maybe pillar needs to rethink what/how it does things, We excplicitly list math options as something we do not support because they have no meaning on time types:

nanotime/R/nanotime.R

Lines 515 to 528 in 69c7612

	##' @rdname nanotime
	setMethod("Math", c("nanotime"),
	function(x) {
	## this is the same error message that R gives for abs("A")
	stop("non-numeric argument to mathematical function")
	})
	##' @rdname nanotime
	setMethod("Math2", c("nanotime"),
	function(x, digits) {
	## this is the same error message that R gives for round("A")
	stop("non-numeric argument to mathematical function")
	})

We represent time as numeric offsets from an epoch. Negative values are permitted, they signal earlier times:

> xx <- as.nanotime(0) + c(-1,1) * 1e9
> xx
[1] 1969-12-31T23:59:59+00:00 1970-01-01T00:00:01+00:00
> 

Permitting abs() makes no sense, it also does not work on POSIXct:

> pp <- as.POSIXct(0, origin="1970-01-01", tz="UTC") + c(-1,1)*60
> pp
[1] "1969-12-31 23:59:00 UTC" "1970-01-01 00:01:00 UTC"
> abs(pp)
Error in Math.POSIXt(pp) : 'abs' not defined for "POSIXt" objects
> 

which is the same behaviour we have for nanotime. You can however show() and format() and ... them to your heart's content. Maybe pillar can lean into that instead?

> xx <- as.nanotime(0) + c(-1,1) * 1e9
> xx
[1] 1969-12-31T23:59:59+00:00 1970-01-01T00:00:01+00:00
> abs(xx)
Error in abs(xx) : non-numeric argument to mathematical function
> format(xx)
[1] "1969-12-31T23:59:59+00:00" "1970-01-01T00:00:01+00:00"
> show(xx)
[1] 1969-12-31T23:59:59+00:00 1970-01-01T00:00:01+00:00
> print(xx)
[1] 1969-12-31T23:59:59+00:00 1970-01-01T00:00:01+00:00
> 

[travis-leith]

I think that all makes sense. I will close this, at least it serves as a discussion point for the pillar issue.

[eddelbuettel]

I can understand your frustration, and I appreciate that you understand that we cannot willy-nilly add an interface here. If pillar does clever thinks with scaling or whatever (I am unsure what "shafting" is mean to imply) maybe it needs to carefully recast into integer64 first and then reset, or some other (local !) measure? This may have come up for them before, and I concur that it is better handled over there.

[eddelbuettel]

Also see #31 and maybe check in with @ijlyttle -- I do have a vague recollection that we ended up printing fine with tibble variants of data.frame objects at some point but I don't have a set of frozen Docker container with tibble and nanotime snapshots to check....

[krlmlr]

Works now in the dev version of pillar with r-lib/pillar#380.

[eddelbuettel]

That is welcome news! I didn't have time to dig through older version to confirm but I think we previously had the ability to use sich nanotime columns. I could of course be wrong.

[DavisVaughan]

I suppose this is really a question of whether or not nanotime should "contain"/inherit from integer64, or if it should just be a slot. Since is.numeric(<Date>) returns FALSE, I think I would have expected is.integer64(<nanotime>) to return FALSE, but that isn't the case:

# this is FALSE
is.numeric(Sys.Date())
#> [1] FALSE

# because...
is.numeric.Date
#> function (x) 
#> FALSE
#> <bytecode: 0x7fec1a030a80>
#> <environment: namespace:base>

# so i would have expected FALSE here 
bit64::is.integer64(nanotime::nanotime(Sys.Date()))
#> [1] TRUE

This returns TRUE because nanotime "contains" an integer64 object, making the S3 dispatch class hierarchy look like c("nanotime, "integer64") rather than just c("nanotime").

nanotime/R/nanotime.R

Line 2 in 02f191d

setClass("nanotime", contains = "integer64")

So you end up being able to dispatch on nanotime objects when you write integer64 S3 methods, which is what went wrong in pillar.

foo <- function(x) {
  UseMethod("foo")
}
foo.default <- function(x) {
  "default"
}
foo.integer64 <- function(x) {
  "s3-method"
}

data <- bit64::as.integer64(1)

# With inheritance
setClass("s4_part1", contains = "integer64")
x <- new("s4_part1", data)

foo(x)
#> [1] "s3-method"

# Without inheritance
s4_part2 <- setClass("s4_part2", slots = c(data = "integer64"))
x <- s4_part2(data = data)

foo(x)
#> [1] "default"

[eddelbuettel]

Thanks for digging where it went wrong for pillar, and the compact example.

We have in the past made use of the fact that we inherit from integer64 -- as I recall this provided fallback if/when we did not fill in all behaviour. I think we are more feature complete now, and the slot idea could be used. It may also be internal enough to be shielded. But I am not sure if it is worth any potential breakage from changing it now; we would have to look into that.

[ijlyttle]

checking in very late; glad to see it got sorted out (far better than I could have suggested), and happy new year to all!