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

summary fails on a nanotime vector #24

Closed
lsilvest opened this issue Jun 3, 2017 · 6 comments
Closed

summary fails on a nanotime vector #24

lsilvest opened this issue Jun 3, 2017 · 6 comments

Comments

@lsilvest
Copy link
Collaborator

@lsilvest lsilvest commented Jun 3, 2017

summary(nanotime((1:100)*3600e9))
                                 Min.                               1st Qu. 
"1970-01-01T00:00:00.000000000+00:00" "1970-01-01T00:00:00.000000000+00:00" 
                               Median                                  Mean 
"1970-01-01T00:00:00.000000000+00:00" "1970-01-01T00:00:00.000000000+00:00" 
                              3rd Qu.                                  Max. 
"1970-01-01T00:00:00.000000000+00:00" "1970-01-01T00:00:00.000000000+00:00" 
Warning message:
In qtile.integer64(x, probs = probs, na.rm = na.rm, names = names,  :
  class 'nanotime' has no 'names' slot; assigning a names attribute will create an invalid object

(Edited: You want three opening ticks followed by the language, here r. --Dirk)

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jun 3, 2017

Not nice. I guess we need a new S4 method?

@lsilvest
Copy link
Collaborator Author

@lsilvest lsilvest commented Jun 4, 2017

Yes, will look into it.

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jun 6, 2017

After #25 it does it as int64, but not always:

R> options("width"=70)
R> summary(nanotime(1:10))
                                 Min. 
"1970-01-01T00:00:00.000000001+00:00" 
                              1st Qu. 
"1970-01-01T00:00:00.000000003+00:00" 
                               Median 
"1970-01-01T00:00:00.000000006+00:00" 
                                 Mean 
"1970-01-01T00:00:00.000000005+00:00" 
                              3rd Qu. 
"1970-01-01T00:00:00.000000008+00:00" 
                                 Max. 
"1970-01-01T00:00:00.000000010+00:00" 
R> summary(nanotime(1:10)*3.6e6)
integer64
    Min.  1st Qu.   Median     Mean  3rd Qu.     Max. 
 3600000 10800000 21600000 19800000 28800000 36000000 
R> 
@lsilvest
Copy link
Collaborator Author

@lsilvest lsilvest commented Jun 7, 2017

What's going on above is that multiplying a nanotime by a double reverts to an integer64. The following works as intended:

> summary(nanotime(1:10*3.6e6))
                                 Min.                               1st Qu. 
"1970-01-01T00:00:00.003600000+00:00" "1970-01-01T00:00:00.010800000+00:00" 
                               Median                                  Mean 
"1970-01-01T00:00:00.021600000+00:00" "1970-01-01T00:00:00.019800000+00:00" 
                              3rd Qu.                                  Max. 
"1970-01-01T00:00:00.028800000+00:00" "1970-01-01T00:00:00.036000000+00:00" 

I would say we should raise an error on multiplication and division of a nanotime, like POSIXct does:

> Sys.time()*3
Error in Ops.POSIXt(as.POSIXct(Sys.time()), 3) : 
  '*' not defined for "POSIXt" objects

What do you think?

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jun 7, 2017

That's probably reasonable, and doable.

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jun 15, 2017

Nicely done:

R> summary(nanotime((1:100)*3600e9))
                                 Min.                               1st Qu. 
"1970-01-01T01:00:00.000000000+00:00" "1970-01-02T02:00:00.000000000+00:00" 
                               Median                                  Mean 
"1970-01-03T02:00:00.000000000+00:00" "1970-01-03T02:30:00.000000000+00:00" 
                              3rd Qu.                                  Max. 
"1970-01-04T03:00:00.000000000+00:00" "1970-01-05T04:00:00.000000000+00:00" 
R> 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.