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

r_make() does not pick up changes in from knitr_in() #887

Closed
3 tasks done
pat-s opened this issue May 27, 2019 · 7 comments
Closed
3 tasks done

r_make() does not pick up changes in from knitr_in() #887

pat-s opened this issue May 27, 2019 · 7 comments

Comments

@pat-s
Copy link
Member

pat-s commented May 27, 2019

Prework

Description

When changing the dependencies of an Rmarkdown report tracked with knitr_in(), r_make() (and the manual way sourcing the config) sometimes do not pick up the following changes to dependencies loaded with drake::loadd():

  • changes to object names
  • commented/uncommented objects

This sounds in the first place like a trigger related problem.
However, this happens in an inconsistent fashion, i.e. the behavior is not persistent across the above mentioned actions.
Neither it is persistent across R sessions.
I've tried to find a reproducible pattern but was not successful.

A reprex seems to be complicated here since I can't interactively change the object name and then source the config again.
It could be related to triggers?
But I would expect that changing object names of dependencies of a report file changes the hash of the drake target.

Reproducible example

Not available.

Environment

Tested on v7.2.0 but I see no possible related changes to knitr_in() in the latest release.

Expected output

Changes of dependency object names in Rmarkdown reports get picked up by drake.

@wlandau
Copy link
Collaborator

wlandau commented May 27, 2019

I think this is because the processing steps in drake_config() are memoized (see create_drake_layout()). What this means is that some dependency checks are skipped if drake thinks they do not need to be repeated. If I am right, all we need to do is memoize against the relevant knitr / R Markdown source files. I will take a look tonight.

wlandau pushed a commit that referenced this issue May 27, 2019
@wlandau
Copy link
Collaborator

wlandau commented May 27, 2019

A reprex:

library(drake)
  plan <- drake_plan(
  a = TRUE,
  b = TRUE,
  report_step = knitr_in("report.Rmd")
)

# Two versions of the report.
lines_a <- c(
  "---",
  "title: abc",
  "---",
  "",
  "```{r}",
  "readd(a)",
  "```"
)
lines_b <- c(
  "---",
  "title: abc",
  "---",
  "",
  "```{r}",
  "readd(b)",
  "```"
)

# Compute the DAG on the first report.
writeLines(lines_a, "report.Rmd")
cache <- storr::storr_environment()
for (i in 1:2) {
  config <- drake_config(
    plan,
    cache = cache,
    session_info = FALSE
  )
}
vis_drake_graph(config)

# drake does not recompute the DAG
# when the report has different dependencies.
writeLines(lines_b, "report.Rmd")
config <- drake_config(
  plan,
  cache = cache,
  session_info = FALSE
)

# The report step should depend on `b`, not `a`.
vis_drake_graph(config)

Created on 2019-05-27 by the reprex package (v0.3.0)

@wlandau
Copy link
Collaborator

wlandau commented May 27, 2019

Yup, it was the memoization. Patch coming soon.

@wlandau
Copy link
Collaborator

wlandau commented May 27, 2019

Now fixed.

@pat-s
Copy link
Member Author

pat-s commented May 28, 2019

Thanks for the instant fix, works like a charm!
Impressive that you even came up with a reprex for this non-trivial case. 🚀

@wlandau
Copy link
Collaborator

wlandau commented May 28, 2019

Thanks.

I may have actually introduced a bug, so I am reopening this. I need to check what happens if the knitr files are missing.

@wlandau
Copy link
Collaborator

wlandau commented May 28, 2019

Reproduced, fixed, and tested in 8adaee9.

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

No branches or pull requests

2 participants