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

shift() with data type integer64 #4865

Closed
peterlittlejohn opened this issue Jan 2, 2021 · 4 comments · Fixed by #5189
Closed

shift() with data type integer64 #4865

peterlittlejohn opened this issue Jan 2, 2021 · 4 comments · Fixed by #5189
Labels
Milestone

Comments

@peterlittlejohn
Copy link

Hello data.table team,

Here's an issue with the shift operator when used with data type integer64.

With type integer there are no issues:

d1 <- data.table(a=1:4)
d1[,b:=shift(a,fill=0L)]
d1[]                                        ## works as expected
   a b
1: 1 0
2: 2 1
3: 3 2
4: 4 3

Now with type integer64:

library(bit64)
d2 <- data.table(a=as.integer64(1:4))
d2[,b:=shift(a,fill=as.integer64(0))]        ## as.integer64("0") or as.integer64(0L) does not work either
Error in shift(a, fill = as.integer64(0)) : 
  INTEGER() can only be applied to a 'integer', not a 'double'

Strictly speaking, the data type of whatever we're filling with should be of the type of the rest of the vector. But this works:

d2 <- data.table(a=as.integer64(1:4))
d2[,b:=shift(a,fill=0L)]
d2[]
   a b
1: 1 0
2: 2 1
3: 3 2
4: 4 3

> sapply(d2,class)
          a           b 
"integer64" "integer64" 

Maybe it should throw an error instead?

> sessionInfo()
R version 4.0.2 (2020-06-22)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 18363)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252   
[3] LC_MONETARY=English_United States.1252 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] bit64_4.0.5       bit_4.0.4         data.table_1.13.0

loaded via a namespace (and not attached):
[1] compiler_4.0.2

I have the same issue on a linux box with older versions of R and data.table.

@jangorecki
Copy link
Member

Thanks for reporting. It should work. Very similar logic is already in nafill function, and there is an improvement pending to have internal, int64 aware, coerceAs function to be used in nafill, #4586. It is probably the best to utilize it in shift as well.

@peterlittlejohn

This comment has been minimized.

@jangorecki
Copy link
Member

jangorecki commented Jan 3, 2021

When you skip i argument then you do not **sub-**assign to b anymore but you are overriding it (assigning to). Therefore changing class of the object is possible. Moreover ifelse is known to be type unstable, by design, so behavior you are reporting is not surprising.
You can try to replace ifelse with fifelse which is faster and type stable alternative.

@peterlittlejohn
Copy link
Author

Ah, yes! Indeed, this works:
d2[,b:=fifelse(b==as.integer64(0),as.integer64(10),b)]
a b
1: 1 10
2: 2 1
3: 3 2
4: 4 3

sapply(d2,class)
a b
"integer64" "integer64"

Thank you.

The issue with shift() and fill with type integer64 still seems to stand of course.

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

Successfully merging a pull request may close this issue.

3 participants