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

Fixing bug when a config only points to other config files and Switch to using simple glob instead of regexp #119

Merged
merged 8 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading