Merge pull request #200 from mutlusun/fix/quarto-identify-source-files

Fix/quarto identify source files

.github/workflows/check.yaml

@@ -31,7 +31,7 @@ jobs:
 
       - uses: r-lib/actions/setup-pandoc@v2
 
-      - uses: quarto-dev/quarto-actions/setup@v2
+      - uses: quarto-dev/quarto-actions/setup@v2.1.6
 
       - name: Install Mac system dependencies
         if: runner.os == 'macOS'

NEWS.md

@@ -4,6 +4,7 @@
 * Detect child quarto documents (#199, @mutlusun).
 * Improve reporting of static branch names from `tar_map()` and `tar_map_rep()` (#201, @kkmann).
 * Ensure compatibility with `targets` after https://github.com/ropensci/targets/issues/1368.
+* Improve detection of Quarto files in `tar_quarto_inspect()` (#200, @multusun).
 
 # tarchetypes 0.10.0
 

R/tar_quarto_files.R

@@ -11,7 +11,8 @@
 #'   * `output`: output files that will be generated during
 #'     `quarto::quarto_render()`.
 #'   * `input`: pre-existing files required to render the project or document,
-#'     such as `_quarto.yml`.
+#'     such as `_quarto.yml` and quarto extensions.
+#' @inheritParams quarto::quarto_render
 #' @param path Character of length 1, either the file path
 #'   to a Quarto source document or the directory path
 #'   to a Quarto project. Defaults to the Quarto project in the
@@ -33,7 +34,7 @@
 #' writeLines(lines, path)
 #' # If Quarto is installed, run:
 #' # tar_quarto_files(path)
-tar_quarto_files <- function(path = ".", profile = NULL) {
+tar_quarto_files <- function(path = ".", profile = NULL, quiet = TRUE) {
   assert_quarto()
   targets::tar_assert_scalar(path)
   targets::tar_assert_chr(path)
@@ -47,19 +48,22 @@ tar_quarto_files <- function(path = ".", profile = NULL) {
   }
   out <- if_any(
     fs::is_dir(path),
-    tar_quarto_files_project(path),
-    tar_quarto_files_document(path)
+    tar_quarto_files_project(path, quiet),
+    tar_quarto_files_document(path, quiet)
   )
   for (field in c("sources", "output", "input")) {
-    out[[field]] <- sort(fs::path_rel(unique(as.character(out[[field]]))))
-    out[[field]] <- as.character(out[[field]])
+    out[[field]] <- as.character(fs::path_rel(out[[field]]))
+    out[[field]] <- unique(sort(out[[field]]))
   }
   out
 }
 
-tar_quarto_files_document <- function(path) {
-  info <- quarto::quarto_inspect(input = path)
-  out <- list(sources = path)
+tar_quarto_files_document <- function(path, quiet) {
+  info <- quarto::quarto_inspect(input = path, quiet = quiet)
+  out <- list()
+
+  # Collect data about source files.
+  out$sources <- tar_quarto_files_get_source_files(info$fileInformation)
 
   # Collect data about output files.
   for (format in info$formats) {
@@ -69,25 +73,20 @@ tar_quarto_files_document <- function(path) {
     )
   }
 
-  # Collect data about input files.
-  for (myfile in names(info$fileInformation)) {
-    out$input <- c(
-      out$input,
-      # `myfile` are relative paths starting from `path`.
-      myfile,
-      # `includeMap` contains relative paths starting from `myfile`.
-      file.path(
-        dirname(path),
-        info$fileInformation[[myfile]]$includeMap$target
-      )
-    )
+  # Collect data about input files. As this is not a project, there doesn't
+  # exist the `info$files` key. However, we can include resources if present.
+  if (length(info$resources) > 0) {
+    out$input <- info$resources
+    out$input <- out$input[file.exists(out$input)]
+  } else {
+    out$input <- character(0L)
   }
 
   out
 }
 
-tar_quarto_files_project <- function(path) {
-  info <- quarto::quarto_inspect(input = path)
+tar_quarto_files_project <- function(path, quiet) {
+  info <- quarto::quarto_inspect(input = path, quiet)
   targets::tar_assert_nonempty(
     info$config$project$`output-dir`,
     paste(
@@ -97,29 +96,59 @@ tar_quarto_files_project <- function(path) {
     )
   )
 
-  # Detect source files.
-  sources <- info$files$input[file.exists(info$files$input)]
+  out <- list(output = file.path(path, info$config$project$`output-dir`))
+
+  # Collect data about source files.
+  out$sources <- tar_quarto_files_get_source_files(info$fileInformation)
 
-  # Detect input files.
-  input <- info$files
-  input <- unlist(input)
-  input <- input[file.exists(input)]
-  for (myfile in names(info$fileInformation)) {
-    input <- c(
-      input,
-      # `myfile` is an absolute path.
+  # Detect input files like the config file (`_quarto.yml`) and resources like
+  # quarto extensions. Make sure in the end that these files exist.
+  out$input <- unlist(
+    c(
+      info$files$config,
+      info$files$resources
+    )
+  )
+  out$input <- out$input[file.exists(out$input)]
+
+  out
+}
+
+#' @title Get Source Files From Quarto Inspect
+#' @description Collects all files from the
+#'   `fileInformation` field that are used in the current report.
+#' @details `fileInformation` contains a list of files. Each file entry contains
+#'   two data frames. The first, `includeMap`, contains a `source` column (files
+#'   that include other files, e.g. the main report file) and a `target` column
+#'   (files that get included by the `source` files). The `codeCells` data frame
+#'   contains all code cells from the files represented in `includeMap`.
+#' @return A character vector of Quarto source files.
+#' @param file_information The `fileInformation` element of the list
+#'   returned by `quarto::quarto_inspect()`.
+tar_quarto_files_get_source_files <- function(file_information) {
+  ret <- character(0)
+
+  for (myfile in names(file_information)) {
+    # Collect relevant source files. The files in `includeMap$target` are always
+    # relative to the main entry point of the report. Thus, we need to add the
+    # corresponding paths to the entries.
+    #
+    # We don't need to include the `source` column as all files are also present
+    # in `target` or are `myfile`.
+    ret <- c(
+      ret,
       myfile,
-      # `includeMap` files are relative starting from `myfile`.
       file.path(
         dirname(myfile),
-        info$fileInformation[[myfile]]$includeMap$target
+        file_information[[myfile]]$includeMap$target
       )
     )
   }
 
-  list(
-    sources = sources,
-    output = file.path(path, info$config$project$`output-dir`),
-    input = input
-  )
+  # Check that these files actually exist.
+  ret <- ret[file.exists(ret)]
+
+  # We don't need to call `unique` here on `ret` as this happens on the main
+  # function.
+  ret
 }

R/tar_quarto_raw.R

@@ -58,7 +58,7 @@ tar_quarto_raw <- function(
   targets::tar_assert_scalar(profile %|||% ".")
   targets::tar_assert_chr(profile %|||% ".")
   targets::tar_assert_nzchar(profile %|||% ".")
-  info <- tar_quarto_files(path = path, profile = profile)
+  info <- tar_quarto_files(path = path, profile = profile, quiet = quiet)
   sources <- info$sources
   output <- info$output
   if (!is.null(output_file)) {

R/tar_quarto_rep_raw.R

@@ -59,7 +59,10 @@ tar_quarto_rep_raw <- function(
   rep_workers <- as.integer(rep_workers)
   name_params <- paste0(name, "_params")
   sym_params <- as.symbol(name_params)
-  default_output_file <- utils::head(tar_quarto_files(path)$output, n = 1L)
+  default_output_file <- utils::head(
+    tar_quarto_files(path, quiet = quiet)$output,
+    n = 1L
+  )
   default_output_file <- default_output_file %||%
     fs::path_ext_set(path, "html")
   target_params <- targets::tar_target_raw(

tests/testthat/helper-utils.R

@@ -6,9 +6,12 @@ expect_equiv <- function(object, expected, ...) {
 
 skip_quarto <- function() {
   skip_pandoc()
-  skip_if_not_installed("quarto")
-  if (is.null(quarto::quarto_path())) {
-    skip("Quarto not found.")
+  skip_if_not_installed("quarto", minimum_version = "1.4.4")
+  quarto_version <- as.character(quarto::quarto_version())
+  no_quarto <- is.null(quarto::quarto_path()) ||
+    utils::compareVersion(quarto_version, "1.6.33") < 0L
+  if (no_quarto) {
+    skip("Quarto >= 1.6.33 not found.")
   }
 }
 

tests/testthat/test-tar_quarto.R

@@ -482,3 +482,129 @@ targets::tar_test("tar_quarto() creates custom output file", {
   expect_false(file.exists(file.path("report.html")))
   expect_true(file.exists(file.path("test.html")))
 })
+
+targets::tar_test("tar_quarto() reruns if target changes in included file", {
+  skip_quarto()
+  lines <- c(
+    "---",
+    "title: main",
+    "output_format: html",
+    "---",
+    "",
+    "{{< include \"file1.qmd\" >}}"
+  )
+  writeLines(lines, "main.qmd")
+  lines <- c(
+    "# First File",
+    "",
+    "Contains a code cell with a target.",
+    "",
+    "```{r}",
+    "targets::tar_read(data)",
+    "```"
+  )
+  writeLines(lines, "file1.qmd")
+  targets::tar_script({
+    library(tarchetypes)
+    list(
+      tar_target(data, data.frame(x = seq_len(26L), y = letters)),
+      tar_quarto(report, path = "main.qmd")
+    )
+  })
+  # First run.
+  suppressMessages(targets::tar_make(callr_function = NULL))
+  progress <- targets::tar_progress()
+  progress <- progress[progress$progress != "skipped", ]
+  expect_equal(sort(progress$name), sort(c("data", "report")))
+  out <- targets::tar_read(report)
+  out <- setdiff(out, "main_files")
+  if (identical(tolower(Sys.info()[["sysname"]]), "windows")) {
+    expect_equal(
+      # On the windows CI, there seems to be issues and `fs::path_rel`. Because
+      # of that, `file1.qmd` is returned twice and we use `unique` here. On
+      # other platforms this issue does not exist.
+      #
+      # See https://github.com/ropensci/tarchetypes/pull/200 for a discussion.
+      sort(unique(basename(out))),
+      sort(c("main.html", "main.qmd", "file1.qmd"))
+    )
+  } else {
+    expect_equal(
+      sort(basename(out)),
+      sort(c("main.html", "main.qmd", "file1.qmd"))
+    )
+  }
+  # Should not rerun the report.
+  suppressMessages(targets::tar_make(callr_function = NULL))
+  progress <- targets::tar_progress()
+  progress <- progress[progress$progress != "skipped", ]
+  expect_equal(nrow(progress), 0L)
+  # Should rerun the report.
+  targets::tar_script({
+    library(tarchetypes)
+    list(
+      tar_target(data, data.frame(x = rev(seq_len(26L)), y = letters)),
+      tar_quarto(report, path = "main.qmd")
+    )
+  })
+  suppressMessages(targets::tar_make(callr_function = NULL))
+  expect_equal(sort(targets::tar_progress()$name), sort(c("data", "report")))
+})
+
+targets::tar_test("tar_quarto() reruns if an included file changes", {
+  skip_quarto()
+  lines <- c(
+    "---",
+    "title: main",
+    "output_format: html",
+    "---",
+    "",
+    "{{< include \"file1.qmd\" >}}"
+  )
+  writeLines(lines, "main.qmd")
+  lines <- c(
+    "# First File",
+    "",
+    "Includes another file.",
+    "",
+    "{{< include \"file2.qmd\" >}}"
+  )
+  writeLines(lines, "file1.qmd")
+  lines <- c(
+    "# Second File",
+    "",
+    "Does not include another file."
+  )
+  writeLines(lines, "file2.qmd")
+  targets::tar_script({
+    library(tarchetypes)
+    list(
+      tar_quarto(report, path = "main.qmd")
+    )
+  })
+  # First run.
+  suppressMessages(targets::tar_make(callr_function = NULL))
+  progress <- targets::tar_progress()
+  progress <- progress[progress$progress != "skipped", ]
+  expect_equal(sort(progress$name), sort(c("report")))
+  out <- targets::tar_read(report)
+  out <- setdiff(out, "main_files")
+  expect_equal(
+    sort(basename(out)),
+    sort(c("main.html", "main.qmd", "file1.qmd", "file2.qmd"))
+  )
+  # Should not rerun the report.
+  suppressMessages(targets::tar_make(callr_function = NULL))
+  progress <- targets::tar_progress()
+  progress <- progress[progress$progress != "skipped", ]
+  expect_equal(nrow(progress), 0L)
+  # Should rerun the report.
+  lines <- c(
+    "# Second File",
+    "",
+    "A change to the second file."
+  )
+  writeLines(lines, "file2.qmd")
+  suppressMessages(targets::tar_make(callr_function = NULL))
+  expect_equal(sort(targets::tar_progress()$name), sort(c("report")))
+})