Skip to content

Commit e09d912

Browse files
Refactor cedta() for readability (#7162)
* refactor cedta rules into helper * implement for bug * test, NEWS * About 100 packages define this now, no sense keeping track * trailing ws * rearrange: .datatable.aware to the top * push isNamespace check into helper * Fix: look up another level, use %iscall% * nocov very-difficult-to-test lines * more nocov * apply data.table coding style exchange <- with = for assignment --------- Co-authored-by: Benjamin Schwendinger <[email protected]>
1 parent 1ca9cf6 commit e09d912

File tree

1 file changed

+40
-23
lines changed

1 file changed

+40
-23
lines changed

R/cedta.R

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
cedta.pkgEvalsUserCode = c("gWidgetsWWW","statET","FastRWeb","slidify","rmarkdown","knitr","ezknitr","IRkernel", "rtvs")
32
# These packages run user code in their own environment and thus do not
43
# themselves Depend or Import data.table. knitr's eval is passed envir=globalenv() so doesn't
@@ -26,12 +25,10 @@ cedta.pkgEvalsUserCode = c("gWidgetsWWW","statET","FastRWeb","slidify","rmarkdow
2625

2726
# nocov start: very hard to reach this within our test suite -- the call stack within a call generated by e.g. knitr
2827
# for loop, not any(vapply_1b(.)), to allow early exit
29-
.any_eval_calls_in_stack = function() {
30-
calls = sys.calls()
28+
.any_eval_calls_in_stack = function(calls) {
3129
# likelier to be close to the end of the call stack, right?
3230
for (ii in length(calls):1) { # nolint: seq_linter. rev(seq_len(length(calls)))? See https://bugs.r-project.org/show_bug.cgi?id=18406.
33-
the_call = calls[[ii]][[1L]]
34-
if (is.name(the_call) && (the_call %chin% c("eval", "evalq"))) return(TRUE)
31+
if (calls[[ii]] %iscall% c("eval", "evalq")) return(TRUE)
3532
}
3633
FALSE
3734
}
@@ -47,6 +44,42 @@ cedta.pkgEvalsUserCode = c("gWidgetsWWW","statET","FastRWeb","slidify","rmarkdow
4744
FALSE
4845
}
4946

47+
# in a helper to promote readability
48+
# NB: put the most common and recommended cases first for speed
49+
.cedta_impl_ = function(ns, n) {
50+
if (!isNamespace(ns)) {
51+
# e.g. DT queries at the prompt (.GlobalEnv) and knitr's eval(,envir=globalenv()) but not DF[...] inside knitr::kable v1.6
52+
return(TRUE)
53+
}
54+
55+
nsname = getNamespaceName(ns)
56+
if (nsname == "data.table") return(TRUE)
57+
58+
if ("data.table" %chin% names(getNamespaceImports(ns))) return(TRUE) # nocov
59+
60+
if (isTRUE(ns$.datatable.aware)) return(TRUE) # nocov
61+
62+
sc = sys.calls()
63+
if (nsname == "utils") {
64+
if (exists("debugger.look", parent.frame(n+1L))) return(TRUE) # nocov
65+
66+
# 'example' for #2972
67+
if (length(sc) >= 8L && sc[[length(sc) - 7L]] %iscall% 'example') return(TRUE) # nocov
68+
}
69+
70+
if (nsname == "base") {
71+
if (all(c("FUN", "X") %chin% ls(parent.frame(n)))) return(TRUE) # lapply
72+
73+
if (.any_sd_queries_in_stack(sc)) return(TRUE) # e.g. lapply() where "piped-in" j= arg has .SD[]
74+
}
75+
76+
if (nsname %chin% cedta.pkgEvalsUserCode && .any_eval_calls_in_stack(sc)) return(TRUE) # nocov
77+
78+
# both ns$.Depends and get(.Depends,ns) are not sufficient
79+
pkg_ns = paste("package", nsname, sep=":")
80+
tryCatch("data.table" %chin% get(".Depends", pkg_ns, inherits=FALSE), error=function(e) FALSE)
81+
}
82+
5083
# cedta = Calling Environment Data.Table-Aware
5184
cedta = function(n=2L) {
5285
# Calling Environment Data Table Aware
@@ -55,26 +88,10 @@ cedta = function(n=2L) {
5588
return(env$.datatable.aware)
5689
}
5790
ns = topenv(env)
58-
if (!isNamespace(ns)) {
59-
# e.g. DT queries at the prompt (.GlobalEnv) and knitr's eval(,envir=globalenv()) but not DF[...] inside knitr::kable v1.6
60-
return(TRUE)
61-
}
62-
nsname = getNamespaceName(ns)
63-
sc = sys.calls()
64-
ans = nsname=="data.table" ||
65-
"data.table" %chin% names(getNamespaceImports(ns)) || # most common and recommended cases first for speed
66-
(nsname=="utils" &&
67-
(exists("debugger.look", parent.frame(n+1L)) ||
68-
(length(sc)>=8L && sc[[length(sc)-7L]] %iscall% 'example')) ) || # 'example' for #2972
69-
(nsname=="base" && # lapply
70-
(all(c("FUN", "X") %chin% ls(parent.frame(n))) ||
71-
.any_sd_queries_in_stack(sc))) ||
72-
(nsname %chin% cedta.pkgEvalsUserCode && .any_eval_calls_in_stack()) ||
73-
isTRUE(ns$.datatable.aware) || # As of Sep 2018: RCAS, caretEnsemble, dtplyr, rstanarm, rbokeh, CEMiTool, rqdatatable, RImmPort, BPRMeth, rlist
74-
tryCatch("data.table" %chin% get(".Depends",paste("package",nsname,sep=":"),inherits=FALSE),error=function(e)FALSE) # both ns$.Depends and get(.Depends,ns) are not sufficient
91+
ans = .cedta_impl_(ns, n+1L)
7592
if (!ans && getOption("datatable.verbose")) {
7693
# nocov start
77-
catf("cedta decided '%s' wasn't data.table aware. Here is call stack with [[1L]] applied:\n", nsname)
94+
catf("cedta decided '%s' wasn't data.table aware. Here is call stack with [[1L]] applied:\n", getNamespaceName(ns))
7895
print(sapply(sys.calls(), `[[`, 1L))
7996
# nocov end
8097
# so we can trace the namespace name that may need to be added (very unusually)

0 commit comments

Comments
 (0)