Skip to content

Commit

Permalink
close #1403: use a much more conservative strategy to find global var…
Browse files Browse the repository at this point in the history
…iables in a chunk (basically consider all variables to be global)
  • Loading branch information
yihui committed May 19, 2017
1 parent 764580c commit 029838c
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 4 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# CHANGES IN knitr VERSION 1.17 (unreleased)

## MAJOR CHANGES

- the automatic detection of chunk dependencies (via the chunk option `autodep = TRUE`) is more conservative by default now; chunk B depends on chunk A if _any_ variables in B are created in A, no matter if these variables are local or global in B; you can use the chunk option `cache.globals` to manually provide a vector of variable names that should be considered "global" to avoid the dependency when local variables in B are also found in A (thanks, @knokknok, #1403)

## BUG FIXES

- when a table only has one column, `kable()` does not work in R Markdown documents (thanks, @expersso, https://github.com/yihui/printr/issues/31)
Expand Down
6 changes: 5 additions & 1 deletion R/block.R
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,11 @@ block_exec = function(options) {
# make sure all objects to be saved exist in env
objs = intersect(c(objs, obj.new), ls(env, all.names = TRUE))
if (options$autodep) {
cache$objects(objs, code, options$label, options$cache.path)
# you shall manually specify global object names if find_symbols() is not reliable
cache$objects(
objs, options$cache.globals %n% find_symbols(code), options$label,
options$cache.path
)
dep_auto()
}
if (options$cache < 3) {
Expand Down
13 changes: 10 additions & 3 deletions R/cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ new_cache = function() {
} else lines = x
writeLines(lines, con = path)
}
cache_objects = function(keys, code, label, path) {
cache_objects = function(keys, globals, label, path) {
save_objects(keys, label, valid_path(path, '__objects'))
# find globals in code; may not be reliable
save_objects(find_globals(code), label, valid_path(path, '__globals'))
save_objects(globals, label, valid_path(path, '__globals'))
}

cache_load = function(hash, lazy = TRUE) {
Expand Down Expand Up @@ -116,6 +115,14 @@ known_globals = c(
'{', '[', '(', ':', '<-', '=', '+', '-', '*', '/', '%%', '%/%', '%*%', '%o%', '%in%'
)

# analyze code and find out all possible variables (not necessarily global variables)
find_symbols = function(code) {
if (is.null(code) || length(p <- parse(text = code, keep.source = TRUE)) == 0) return()
p = getParseData(p)
p = p[p$terminal & p$token %in% c('SYMBOL', 'SYMBOL_FUNCTION_CALL', 'SPECIAL'), ]
unique(p$text)
}

# a variable name to store the metadata object from code chunks
cache_meta_name = function(hash) sprintf('.%s_meta', hash)
# a variable name to store the text output of code chunks
Expand Down
5 changes: 5 additions & 0 deletions tests/testit/test-cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ assert(
identical(find_globals(c('a=1%*%1%o%2 %in% d', 'b=d%%10+3%/%2-z[1:3]')), c('d', 'z'))
)

assert(
'find_symbols() identifies all symbols',
find_symbols('x = x + 1; rnorm(1, std = z)') %==% c('x', 'rnorm', 'z')
)

knit_lazy = function(lazy = TRUE) {
in_dir(tempdir(), {
txt = c(sprintf('```{r test, cache=TRUE, cache.lazy=%s}', lazy),
Expand Down

0 comments on commit 029838c

Please sign in to comment.