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

anytime is overwriting inputs #68

Closed
christophsax opened this Issue Jun 24, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@christophsax
Copy link

christophsax commented Jun 24, 2017

This is such a useful package, thanks!

But when I use any of the functions on numeric output, it overwrites the input variable for some reason...

It works fine with integer or character input.

Here's an example:

  library(anytime)

  # this works fine
  x <- "1949-01-01"
  b <- anytime(x)
  x  # i.e., x is not modified
#> [1] "1949-01-01"

  x <- -662688000
  b <- anytime(x)
  x  # x is now equal to b !!!
#> [1] "1949-01-01 01:00:00 CET"

  # If I am feeding an integer, everything is fine again...
  x <- -662688000L
  b <- anytime(x)
  x  
#> [1] -662688000
@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jun 24, 2017

Yes, well it is standard R / Rcpp / SEXP-is-a-pointer behaviour. When a SEXP goes in, and we don't need to "cast" it is the variable being carried through.

Here things are a little funny because we either will return a POSIXct (for anytime and variants) or Date.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jun 24, 2017

In the middle case (for numeric) this is unavoidable. We just recast -- the numeric payload is unchanged but we just did a as.POSIXct() so now have a class attribute when before you had not. For the last one it is different as the integer from input becomes a numeric in order to be a POSIXct.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jun 24, 2017

I think the fix would be to add a force clone (ie copy in different memory location) right here: https://github.com/eddelbuettel/anytime/blob/master/src/anytime.cpp#L459

That could be done, I just don't know if it is useful. I presume because as.POSIXct() leaves its input alone, we should too?

R> x <- -662688000 + 3600*6   ## offset for my TZ
R> b <- as.POSIXct(x, origin="1970-01-01")
R> b
[1] "1949-01-01 CST"
R> x
[1] -662666400
R> 
@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jun 24, 2017

Fix coming right up in a branch-that-will-be-a-PR-soon:

R> library(anytime)
R> x <- -662688000 + 3600*6 ; x
[1] -662666400
R> b <- as.POSIXct(x, origin="1970-01-01"); b
[1] "1949-01-01 CST"
R> x
[1] -662666400
R>

eddelbuettel added a commit that referenced this issue Jun 24, 2017

@christophsax

This comment has been minimized.

Copy link
Author

christophsax commented Jun 24, 2017

Nice! Thanks!! 👍

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jun 24, 2017

Looking at my last comment I copied the wrong example:

R> library(anytime)
R> x <- -662688000 + 3600*6 ; x
[1] -662666400
R> b <- anytime(x); b
[1] "1949-01-01 CST"
R> x
[1] -662666400
R> 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.