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

as.character.ITime throws errors on NAs #2171

Closed
franknarf1 opened this issue May 17, 2017 · 4 comments
Closed

as.character.ITime throws errors on NAs #2171

franknarf1 opened this issue May 17, 2017 · 4 comments
Milestone

Comments

@franknarf1
Copy link
Contributor

franknarf1 commented May 17, 2017

[Note: I was mistaken in thinking this was a platform-dependent problem. Just keeping the Windows vs Linux bit so the thread still makes sense. Sorry about that.]

Trying...

as.character(structure(NA_integer_, class = "ITime"))
# on Windows:
# [1] NA

# on Linux
# Error in sprintf("%02d:%02d:%02d", hh, mm, ss) : 
#   invalid format '%02d'; use format %f, %e, %g or %a for numeric objects

This may seem like a minor issue, but it means that I cannot print any data.table containing ITime, like:

data.table(e_time = as.ITime("17:00")[NA_integer_])
# Error in sprintf("%02d:%02d:%02d", hh, mm, ss) : 
#   invalid format '%02d'; use format %f, %e, %g or %a for numeric objects

For whatever reason, my table does print after its various := "update join" calls (I'm not really sure why), so my script can't run through.


Why it's happening:

As far as I can tell, this is a valid format, used here:

res = sprintf('%02d:%02d:%02d', hh, mm, ss)

However, ?sprintf warns:

The format string is passed down the OS's sprintf function, and incorrect formats can cause the latter to crash the R process .


Linux session info:

data.table 1.10.5 IN DEVELOPMENT built 2017-05-11 23:55:12 UTC; travis

R version 3.3.1 (2016-06-21)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 16.04 LTS

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

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

other attached packages:
[1] magrittr_1.5      data.table_1.10.5

loaded via a namespace (and not attached):
[1] tools_3.3.1
@MichaelChirico
Copy link
Member

MichaelChirico commented May 18, 2017

This is coming from format.ITime. The issue is ss is becoming NA_real_, and this is an error:

sprintf('%02d:%02d:%02d', NA_integer_, NA_integer_, NA_real_)

However, sprintf('%02d:%02d:%02d', 1L, 1L, 1) is not an error... so I guess this is an R bug?

We could just force the output of trunc to be an integer, but the inconsistency in R is confusing.

@franknarf1 can you confirm that sprintf('%02d:%02d:%02d', NA_integer_, NA_integer_, NA_real_) is not an error on Windows?

@franknarf1
Copy link
Contributor Author

franknarf1 commented May 18, 2017

@MichaelChirico Sorry, I hadn't realized that my Windows data.table 1.10.5 was not recent enough (from 5/9, I think). Now that I have upgraded both platforms to data.table 1.10.5 IN DEVELOPMENT built 2017-05-18 00:04:56 UTC; travis (to get 0e68a72 ), every call in question gives that error on both platforms.

I guess I learned my lesson to always give exhaustive session info. I could have caught my own mistake that way. Anyway, still a bug, just not platform-dependent. I'll edit the title. Thanks

@franknarf1 franknarf1 changed the title as.character.ITime throws errors on NAs on linux as.character.ITime throws errors on NAs May 18, 2017
@MichaelChirico
Copy link
Member

MichaelChirico commented May 18, 2017

@franknarf1 thanks. I still am not sure whether this is a data.table bug or an R bug...

I'll push a fix here. I think ss is forced to be between 0 and 60, so we can just use as.integer instead of trunc per here:

http://stackoverflow.com/questions/43894077/what-is-the-difference-between-trunc-and-as-integer

mattdowle added a commit that referenced this issue May 20, 2017
@mattdowle mattdowle added this to the v1.10.6 milestone May 20, 2017
@MichaelChirico
Copy link
Member

MichaelChirico commented May 25, 2017

Appears this was counted as an R bug -- apparently fixed in latest devel version (72737):

https://stat.ethz.ch/pipermail/r-devel/2017-May/074342.html

I think we should keep as.integer since it's faster anyway, but maybe add a note to the source code referencing this.

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

No branches or pull requests

3 participants