exclude ... from tables() search
#5199
Conversation
R/tables.R
Outdated
| { | ||
| # Prints name, size and colnames of all data.tables in the calling environment by default | ||
| all_obj = objects(envir=env, all.names=TRUE) | ||
| # include "hidden" objects (starting with .) via all.names=TRUE, but exclude ... specifically, #5197 | ||
| all_obj = grep("...", objects(envir=env, all.names=TRUE), invert=TRUE, fixed=TRUE, value=TRUE) |
There was a problem hiding this comment.
btw any reason to use objects() here? it looks like an exact duplicate of ls(), but probably less well-known...
There was a problem hiding this comment.
nope - just I got used to objects(). I didn't know ls() was an exact duplicate of objects(), that's interesting, so I don't mind using ls() instead.
There was a problem hiding this comment.
| is_DT = which(vapply_1b(all_obj, function(x) is.data.table(get(x, envir=env)))) | ||
| if (!length(is_DT)) { | ||
| # include "hidden" objects (starting with .) via all.names=TRUE, but exclude ... specifically, #5197 | ||
| all_obj = grep("...", objects(envir=env, all.names=TRUE, sorted=order.col == "NAME"), invert=TRUE, fixed=TRUE, value=TRUE) |
There was a problem hiding this comment.
we could also pass pattern="^([^.]|[.][^.]|[.][.][^.])" to do it in one call, but that's pretty ugly, and anyway objects() itself is just calling grep() so there's no efficiency benefit (i.e., pattern is not being handled in C), so wrapping in grep() does the same thing with a more readable grep() call.
There was a problem hiding this comment.
objects() only calls grep() when pattern is not missing, iiuc. But pattern is missing in this objects() call. So doesn't adding grep here add overhead of grep when all we want to do is remove the string "..." if it's present?
Codecov Report
@@ Coverage Diff @@
## master #5199 +/- ##
=======================================
Coverage 99.51% 99.51%
=======================================
Files 77 77
Lines 14527 14528 +1
=======================================
+ Hits 14456 14457 +1
Misses 71 71
Continue to review full report at Codecov.
|
| if (!length(is_DT)) { | ||
| # include "hidden" objects (starting with .) via all.names=TRUE, but exclude ... specifically, #5197 | ||
| all_obj = grep("...", objects(envir=env, all.names=TRUE, sorted=order.col == "NAME"), invert=TRUE, fixed=TRUE, value=TRUE) | ||
| is_DT = vapply_1b(mget(all_obj, envir=env), is.data.table) |
There was a problem hiding this comment.
changing from get to mget here meant that inherits was changed from TRUE to FALSE because the defaults are different. But doesn't matter as the input is ls() in that env so it does exist there. Ok, got it.
Closes #5197