Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subtraction (and other internal methods) for POSIXxt class #13

Closed
shoebodh opened this issue Nov 28, 2023 · 3 comments
Closed

subtraction (and other internal methods) for POSIXxt class #13

shoebodh opened this issue Nov 28, 2023 · 3 comments

Comments

@shoebodh
Copy link

Hi, I ran into a small issue with the new POSIXxt objects stringx introduces where the substraction method doesn't work.

For instance we know this works

aa = base::Sys.time()
bb = base::Sys.time()
bb - aa
Time difference of 0.002556086 secs

But currently the following gives an error

aa = stringx::Sys.time()
bb = stringx::Sys.time()
bb - aa
bb - aa
Error in as.POSIXct.default(x, tz = tz, ...) : 
  do not know how to convert 'x' to class “POSIXct”

I found that this occurs because the class of stringx::Sys.time() is
"POSIXxt" "POSIXct" "POSIXt" so the - doesn't know how to handle it when it finds POSIXxt instead of the internal POSIX classes.

For my case I managed this with a very simple work around by introducing the `-.POSIXxt' S3 method like the following

`-.POSIXxt` <- function(...) difftime(...)
#Now the subtraction works
bb-aa
Time difference of 0.002810955 secs

This works great but I think the internal '-.POSIXxt' is a little more comprehensive so the following also works, but i find this a litte hacky because here I simply revert the class attribute) to trick the subtraction method to find the "POSIXxt" when it looks for that attribute.

`-.POSIXxt` <- function(e1, e2) {
  
  attr(e1, 'class') <- rev(class(e1)) # 
  attr(e2, 'class') <- rev(class(e2))
  
  return( `-`(e1 , e2))
}

bb-aa
Time difference of 0.002810955 secs

These are my naive workarounds, but I am sure there is a more comprehensive and elegant way to add this method in the library.
I think that having having these internal POSIXt methods for the POSIXxt class in the library is important for improved compatibility with the base package.

Thanks

@gagolews
Copy link
Owner

Thanks for pointing this out!

Could you test if the following function does the trick?

`-.POSIXxt` <- function(e1, e2) NextMethod(.Generic)

@shoebodh
Copy link
Author

That works wonderfully. Thank you.

@gagolews
Copy link
Owner

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants