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

[bug] edge case: setnames shouldn't error on null data.tables when assigned character(0) #2452

Closed
arunsrinivasan opened this issue Oct 31, 2017 · 3 comments
Labels
Milestone

Comments

@arunsrinivasan
Copy link
Member

@arunsrinivasan arunsrinivasan commented Oct 31, 2017

require(data.table) # devel built 2017-10-31
dt <- data.table()
attributes(dt)
# $class
# [1] "data.table" "data.frame"
# 
# $row.names
# integer(0)
# 
# $names
# character(0)
# 
# $.internal.selfref
# <pointer: 0x0000000000310788>
# 

setnames(dt, character(0))
# Error in setnames(dt, character(0)) : x has no column names

names(df) <- character(0) would work on a null data.frame. I think this should not error. The reason is that it avoids an unnecessary if (ncol(dt)) before to do setnames(dt, nm) where nm is the vector of names, which would be character(0) if dt had no cols.


Output of sessionInfo()

# R version 3.3.2 (2016-10-31)
# Platform: x86_64-w64-mingw32/x64 (64-bit)
# Running under: Windows Server >= 2012 x64 (build 9200)
# 
# 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] data.table_1.10.5
# 
# loaded via a namespace (and not attached):
# [1] tools_3.3.2 yaml_2.1.14
@MichaelChirico
Copy link
Member

@MichaelChirico MichaelChirico commented Oct 31, 2017

Caused by this line:

if (!length(attr(x, "names"))) stop("x has no column names")  # because setnames is for user user. Internally, use setattr(x,"names",...)

I don't think I understand the comment? Can't this line just be expunged and that's that? test.data.table() runs fine if I do so & rebuild. So at the very least the purpose of that line is absent from the test suite...

@arunsrinivasan
Copy link
Member Author

@arunsrinivasan arunsrinivasan commented Oct 31, 2017

I don't understand the comment and I think that line needn't exist, since the next line ensures that cols of data.table and the input character vector are identical.

@mattdowle mattdowle added this to the v1.10.6 milestone Oct 31, 2017
@arunsrinivasan
Copy link
Member Author

@arunsrinivasan arunsrinivasan commented Oct 31, 2017

👍🏽

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

No branches or pull requests

3 participants