moved key= into ... of as.data.table generic #3852
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3852 +/- ##
=======================================
Coverage 99.42% 99.42%
=======================================
Files 71 71
Lines 13412 13412
=======================================
Hits 13335 13335
Misses 77 77
Continue to review full report at Codecov.
|
| @@ -1,4 +1,5 @@ | |||
| as.data.table = function(x, keep.rownames=FALSE, key=NULL, ...) | |||
| as.data.table = function(x, keep.rownames=FALSE, ...) | |||
| # cannot add new args before dots otherwise revdeps which implement their methods will start to warn; e.g. riskRegression in #3581 | |||
There was a problem hiding this comment.
but why not adding key after dots then? or asking revdep to update. Such a warning is not a problem for production systems
There was a problem hiding this comment.
Adding arguments after ... has some wrinkles in the past. I can't quite remember what the problem is, but isn't ... supposed to be last?
It works, so why add key=NULL after dots?
There was a problem hiding this comment.
lots of base examples with dots in the middle (e.g. sapply)... just means you always have to name that argument.
idea of including is to signal its importance in general for this method. how many as.data.table methods don't accept key?
There was a problem hiding this comment.
arguments that are after ... has to be always match by name, I can't recall anything other.
There was a problem hiding this comment.
there's something about S3 generics as well, can't recall off the top of my head
There was a problem hiding this comment.
For edification: 106 base functions with arguments after ... (including as.data.frame, the closest cousin):
names(Filter(function(x) is.function(x) && '...' %chin% (nm <- names(formals(x))) && tail(nm, 1L) != '...',
+ mget(ls(base <- asNamespace('base')), envir = base)))
[1] "[.Date" "[.difftime" "[.factor"
[4] "[.POSIXct" "[.table" "[[.data.frame"
[7] "[[.Date" "[[.numeric_version" "[[.POSIXct"
[10] "[[<-.factor" "[[<-.numeric_version" "[<-.Date"
[13] "[<-.factor" "[<-.POSIXct" "all.equal.character"
[16] "all.equal.factor" "all.equal.list" "all.equal.numeric"
[19] "all.equal.POSIXt" "all.equal.raw" "as.data.frame.character"
[22] "as.data.frame.complex" "as.data.frame.Date" "as.data.frame.difftime"
[25] "as.data.frame.factor" "as.data.frame.integer" "as.data.frame.list"
[28] "as.data.frame.logical" "as.data.frame.matrix" "as.data.frame.noquote"
[31] "as.data.frame.numeric" "as.data.frame.numeric_version" "as.data.frame.ordered"
[34] "as.data.frame.POSIXct" "as.data.frame.raw" "as.data.frame.table"
[37] "as.data.frame.vector" "attr.all.equal" "by"
[40] "by.data.frame" "by.default" "c.Date"
[43] "c.difftime" "c.noquote" "c.numeric_version"
[46] "c.POSIXct" "c.POSIXlt" "c.warnings"
[49] "cat" "cbind" "cbind.data.frame"
[52] "chkDots" "data.frame" "eapply"
[55] "errorCondition" "expand.grid" "file.create"
[58] "file.info" "file.path" "file.show"
[61] "format.data.frame" "gettext" "gettextf"
[64] "interaction" "mapply" "message"
[67] "namespaceImport" "order" "packageStartupMessage"
[70] "paste" "paste0" "pmax"
[73] "pmax.int" "pmin" "pmin.int"
[76] "print.by" "print.data.frame" "range.default"
[79] "rbind" "rbind.data.frame" "remove"
[82] "requireNamespace" "rm" "sapply"
[85] "save" "split<-" "split<-.data.frame"
[88] "split<-.default" "stop" "stopifnot"
[91] "Summary.data.frame" "Summary.Date" "summary.default"
[94] "Summary.difftime" "Summary.factor" "Summary.numeric_version"
[97] "Summary.ordered" "Summary.POSIXct" "Summary.POSIXlt"
[100] "system.file" "table" "tapply"
[103] "tryCatch" "vapply" "warning"
[106] "warningCondition"
Another 100 or so among the other default loads
Resolves
riskRegressionin #3581