Skip to content

Commit

Permalink
Fixing bug when a config only points to other config files and Switch…
Browse files Browse the repository at this point in the history
… to using simple glob instead of regexp (#119)

* Fixing issue in internal_run() when config points to other config files.

* Switch to using simple glob instead

* Update vignette to reflect the use of simple glob

* Updating news and description

* Updating NEWS and DESCRIPTION

* Set n_workers = 2 in tests

---------

Co-authored-by: Troejelsgaard <[email protected]>
  • Loading branch information
Troejelsgaard and NNktqn authored Dec 2, 2024
1 parent b01df29 commit 9310cc0
Show file tree
Hide file tree
Showing 12 changed files with 35 additions and 33 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: whirl
Title: Logging package
Version: 0.1.5.9000
Version: 0.1.6
Authors@R: c(
person("Aksel", "Thomsen", , "[email protected]", role = c("aut", "cre")),
person("Lovemore", "Gakava", , "[email protected]", role = "aut"),
Expand Down
7 changes: 6 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# whirl dev
# whirl 0.1.6 (2024-11-25)
* Added support for logging of Python scripts with `run()`.
* Improved unit tests for `run()`
* Fixing a bug where the queue was not returned correctly in some instances.
* Switched to using `Sys.glob()` instead of `utils::glob2rx()`.

# whirl 0.1.5 (2024-11-21)
* Reducing the number of dependencies.

# whirl 0.1.4 (2024-11-01)
* Add `use_whirl()` utility function.
Expand Down
2 changes: 1 addition & 1 deletion R/enrich_input.R
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ enrich_input <- function(input, steps = NULL,
# Normalizing the paths and read regexp
for (j in seq_along(paths)) {
normalized <- unlist(lapply(paths[[j]], normalize_with_base, base = root_dir))
paths[[j]] <- read_regexp(normalized)
paths[[j]] <- read_glob(normalized)
}

# If input include one or more directories
Expand Down
8 changes: 2 additions & 6 deletions R/internal_run.R
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,10 @@ internal_run <- function(input, steps, queue, level,
verbosity_level = verbosity_level)
} else {
# Execute the scripts
result <- queue$run(files)
queue$run(files)
}
}

if (exists("result")) {
invisible(result)
}


invisible(queue)
}

12 changes: 5 additions & 7 deletions R/read_regexp.R → R/read_glob.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,17 @@
#' used as input these will be solved to the actual files matching the
#' criteria.
#' @noRd
read_regexp <- function(input) {

read_glob <- function(input) {
files_ <- lapply(input, function(x) {
#If the file exist then return the path
if (file.exists(x)) {
return(x)
} else {
#If the file does not exist then check if it is a regexp
regexp <- basename(x) |>
utils::glob2rx()

files <- list.files(path = dirname(x), pattern = regexp, full.names = TRUE)
#If the file does not exist then check if it is a glob
files <- Sys.glob(x)
if (length(files) == 0) {
cli::cli_abort("No files or folders for this path {x}")
cli::cli_alert_warning("No files or folders for this path {x}")
}
return(files)
}
Expand Down
6 changes: 6 additions & 0 deletions inst/examples/demo/adam/adamprog_whirl.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
steps:
- name: "Run adam mk400 programs"
paths:
- "mk400*.R"


2 changes: 1 addition & 1 deletion tests/testthat/scripts/_whirl_r_programs.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
steps:
- name: "Run all R programs"
paths:
- "*.r|R"
- "*.R"
4 changes: 2 additions & 2 deletions tests/testthat/test-examples.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ test_that("All example scripts run with consistent output", {

# Run all examples after in separate steps

res <- list.files() |>
res <- list(list.files(pattern = "*.yaml"), list.files(pattern = "*.R$")) |>
as.list() |>
run() |>
run(n_workers = 2) |>
expect_no_error() |>
expect_no_warning()

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-internal_run.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ test_that("testing internal_run()", {

# A config file

q <- whirl_queue$new()
q <- whirl_queue$new(n_workers = 2)

test_script("_whirl.yaml") |>
internal_run(steps = NULL, level = 1, queue = q) |>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
test_that("read_regexp() finds the right files", {
test_that("testing read_glob()", {

# A single file

test_script("success.R") |>
read_regexp() |>
read_glob() |>
expect_equal(test_script("success.R"))

# All R files in a directory

test_script("") |>
file.path("*.R") |>
read_regexp() |>
read_glob() |>
expect_match("\\.R$") |>
length() |>
expect_gt(1)

# Error when file does not exist

test_script("") |>
file.path("fake_program.R") |>
read_regexp() |>
expect_error()
read_glob() |>
expect_message()

})
6 changes: 3 additions & 3 deletions tests/testthat/test-run.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test_that("Run single python script", {
test_that("Run multiple R scripts", {

res <- test_script(c("success.R", "warning.R", "error.R")) |>
run() |>
run(n_workers = 2) |>
expect_no_error()

res[["status"]] |>
Expand All @@ -56,7 +56,7 @@ test_that("Run multiple R scripts", {
test_that("Run multiple python scripts", {

res <- test_script(c("py_success.py", "py_warning.py", "py_error.py")) |>
run() |>
run(n_workers = 2) |>
expect_no_error()

res[["status"]] |>
Expand All @@ -81,7 +81,7 @@ test_that("Run multiple python scripts", {
test_that("Run yaml config file", {

res <- test_script("_whirl.yaml") |>
run() |>
run(n_workers = 2) |>
expect_no_error()

})
4 changes: 2 additions & 2 deletions vignettes/whirl.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ The location of the summary file can be controlled with the `summary_file` argum
```{r}
# Execution of all R files in a specific directory
run(
input = "path/to/directory/*.(r|R)",
input = "path/to/directory/*.R",
n_workers = 4,
summary_file = "path/to/summary"
)
# Execution of all R files starting with "mk200" in a specific directory
run(
input = "path/to/directory/mk200*.(r|R)",
input = "path/to/directory/mk200*.R",
n_workers = 8,
summary_file = "path/to/summary"
)
Expand Down

0 comments on commit 9310cc0

Please sign in to comment.