Skip to content

Commit

Permalink
Merge pull request #69 from pharmaverse/61-address-lintr-updates
Browse files Browse the repository at this point in the history
Closes #61: fixed linter config and findings
  • Loading branch information
mstackhouse authored Oct 16, 2024
2 parents 3b52da7 + c5d8084 commit facf9cd
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .lintr
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
linters: with_defaults(line_length_linter(125), object_usage_linter=NULL, cyclocomp_linter(complexity_limit = 20), object_name_linter(styles = c("snake_case", "symbols", "dotted.case")), single_quotes_linter=NULL)
linters: linters_with_defaults(line_length_linter(125), object_usage_linter=NULL, cyclocomp_linter(complexity_limit = 20), object_name_linter(styles = c("snake_case", "symbols", "dotted.case")))
exclusions: list("R/data.R")
24 changes: 12 additions & 12 deletions R/autos.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ set_autos <- function(autos, envsetup_environ = Sys.getenv("ENVSETUP_ENVIRON"))

# Must be named list
if (!is_named(autos)) {
stop("Paths for autos in your envsetup configuration file must be named", call.=FALSE)
stop("Paths for autos in your envsetup configuration file must be named", call. = FALSE)
}

for (i in seq_along(autos)) {
Expand All @@ -30,7 +30,7 @@ set_autos <- function(autos, envsetup_environ = Sys.getenv("ENVSETUP_ENVIRON"))
if (length(cur_autos) > 1) {
# Hierarchical paths must be named
if (!is_named(cur_autos)) {
stop("Hierarchical autos paths in your envsetup configuration file must be named", call.=FALSE)
stop("Hierarchical autos paths in your envsetup configuration file must be named", call. = FALSE)
}

# envsetup_environ must be used if using hierarchical paths
Expand All @@ -43,7 +43,7 @@ set_autos <- function(autos, envsetup_environ = Sys.getenv("ENVSETUP_ENVIRON"))
}

if (!is.null(names(cur_autos)) && !envsetup_environ %in% names(cur_autos)
&& envsetup_environ != ""){
&& envsetup_environ != "") {
warning(paste(
"The", ui_field(names(autos[i])), "autos has named",
"environments", ui_field(names(cur_autos)),
Expand Down Expand Up @@ -105,7 +105,7 @@ attach_auto <- function(path, name) {
if (!(dir.exists(path) || file.exists(path))) {
# Check if the auto actually exists
warning(sprintf("An autos path specified in your envsetup configuration file does not exist: %s = %s", name, path),
call.=FALSE)
call. = FALSE)
} else if (file.exists(path) && !dir.exists(path)) {
# if file, source it
sys.source(path, envir = attach(NULL, name = name_with_prefix))
Expand Down Expand Up @@ -282,13 +282,13 @@ detach_autos <- function() {
#' # see autos are still above purrr in the search path
#' search()
library <- function(..., pos = NULL) {
if(is.null(pos)) {
## we have at least one package loaded (envsetup itself)
## use earliest current package position as place to
## attach all future packages, regardless of what
## envsetup, devtools, or anything else has put
## in front of them
pos <- min(grep("^package:", search()))
if (is.null(pos)) {
## we have at least one package loaded (envsetup itself)
## use earliest current package position as place to
## attach all future packages, regardless of what
## envsetup, devtools, or anything else has put
## in front of them
pos <- min(grep("^package:", search()))
}
base::library(..., pos = pos)
}
}
7 changes: 4 additions & 3 deletions R/init.R
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ init <- function(project, config_path = NULL, create_paths = NULL) {
ui_info(
c("The following paths in your configuration do not exist:",
paths[missing_directories])
)
)

# if not, ask if user would like them built
if (is.null(create_paths)) {
create_paths <-
usethis::ui_yeah(
"Would you like us to create your directories to match your configuration?",
n_no = 1
)
)
}

if (!create_paths) {
Expand Down Expand Up @@ -134,7 +134,8 @@ envsetup_write_rprofile <- function(add, file) {

# if there is a call to `rprofile()` in the .Rprofile, assume setup was already done and exit
if (any(grepl("rprofile\\(", before))) {
warning("It looks like your project has already been initialized to use envsetup. Manually adjust your .Rprofile if you need to change the environment setup.",
warning("It looks like your project has already been initialized to use envsetup.
Manually adjust your .Rprofile if you need to change the environment setup.",
call. = FALSE
)
return(invisible())
Expand Down
2 changes: 1 addition & 1 deletion R/paths.R
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ object_in_path <- function(path, object) {

#' Build directory structure from a configuration file
#'
#' @param config configuration object from config::get() containing paths#'
#' @param config configuration object from config::get() containing paths
#' @param root root directory to build from.
#' Leave as NULL if using absolute paths. Set to working directory if using relative paths.
#'
Expand Down
2 changes: 0 additions & 2 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@
#' validate_config(config::get(file = file.path(tmpdir, "no_paths.yml")))
validate_config <- function(config) {
validate_paths(config)

# validate_autos(config)
}


Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/ref/envre.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ test_envre <- list()

test_envre$PDEV <- get_path(
topdrv, "PDEV", username, compound, protocol, dbrelease, rpteff, "analysis"
)
)

test_envre$PREPROD <- get_path(
topdrv, "PREPROD", NA, compound, protocol, dbrelease, rpteff, "analysis"
)
)

test_envre$PROD <- get_path(
topdrv, "PROD", NA, compound, protocol, dbrelease, rpteff, "analysis"
)
)

envre_loc <- "pdev"
6 changes: 3 additions & 3 deletions tests/testthat/ref/envre_preprod.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ test_envre <- list()

test_envre$PDEV <- get_path(
topdrv, "PDEV", username, compound, protocol, dbrelease, rpteff, "analysis"
)
)

test_envre$PREPROD <- get_path(
topdrv, "PREPROD", NA, compound, protocol, dbrelease, rpteff, "analysis"
)
)

test_envre$PROD <- get_path(
topdrv, "PROD", NA, compound, protocol, dbrelease, rpteff, "analysis"
)
)

envre_loc <- "preprod"
6 changes: 3 additions & 3 deletions tests/testthat/ref/envre_prod.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ test_envre <- list()

test_envre$PDEV <- get_path(
topdrv, "PDEV", username, compound, protocol, dbrelease, rpteff, "analysis"
)
)

test_envre$PREPROD <- get_path(
topdrv, "PREPROD", NA, compound, protocol, dbrelease, rpteff, "analysis"
)
)

test_envre$PROD <- get_path(
topdrv, "PROD", NA, compound, protocol, dbrelease, rpteff, "analysis"
)
)

envre_loc <- "prod"
2 changes: 1 addition & 1 deletion tests/testthat/ref/func.R
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
func <- function() {
print(rlang::env_parent())
}
}
24 changes: 14 additions & 10 deletions tests/testthat/test-autos.R
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,16 @@ test_that("Autos validation from yml happens correctly", {

# Hierarchical list is named
expect_error(
set_autos(list(project = c("path1", "path2"))), "Hierarchical autos paths in your envsetup configuration file must be named"
set_autos(list(project = c("path1", "path2"))),
"Hierarchical autos paths in your envsetup configuration file must be named"
)

# Paths are characters
expect_error(set_autos(list(global = 1)), "Paths provided for autos must be directories")
expect_error(set_autos(list(global = 1)),
"Paths provided for autos must be directories")

expect_warning(set_autos(list(x = "/bad/path/")), "An autos path specified in your envsetup configuration file does not exist")
expect_warning(set_autos(list(x = "/bad/path/")),
"An autos path specified in your envsetup configuration file does not exist")
})

# Detatch and re-setup for QA now
Expand Down Expand Up @@ -108,16 +111,17 @@ test_that("Autos no longer exist when detached", {

test_that("the configuration can be named anything and library will
reattach the autos correctly", {
suppressMessages(rprofile(custom_name))
suppressMessages(rprofile(custom_name))

expect_invisible(suppressPackageStartupMessages(library("purrr")))
expect_invisible(suppressPackageStartupMessages(library("purrr")))

purrr_location <- which(search() == "package:purrr")
autos_locatios <- which(grepl("^autos:", search()))
purrr_location <- which(search() == "package:purrr")
autos_locatios <- which(grepl("^autos:", search()))

expect_true(all(purrr_location > autos_locatios))
detach("package:purrr")
})
expect_true(all(purrr_location > autos_locatios))
detach("package:purrr")
}
)


test_that("Autos warns user when ENVSETUP_ENVIRON does not match named environments in autos", {
Expand Down
15 changes: 10 additions & 5 deletions tests/testthat/test-paths.R
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,16 @@ test_that("1.1", {
#' @editor Aidan Ceney
#' @editDate 2022-05-12
test_that("1.2", {
expect_error(readr::read_csv(read_path(data,
"iris.csv",
full.path = TRUE,
envsetup_environ = "PROD"
)))
expect_error(
readr::read_csv(
read_path(
data,
"iris.csv",
full.path = TRUE,
envsetup_environ = "PROD"
)
)
)
})

#' @editor Aidan Ceney
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-rprofile.R
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ test_that("rprofile stores the configuration", {
stored_config <- base::get(
"auto_stored_envsetup_config",
pos = which(search() == "envsetup:paths")
)
)

expect_equal(custom_name, stored_config)
})
Expand Down
8 changes: 4 additions & 4 deletions vignettes/config.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,11 @@ writeLines(
paste0(
"default:
paths:
data: !expr list(DEV = '",dir,"/demo/DEV/username/project1/data', PROD = '",dir,"/demo/PROD/project1/data')
output: '",dir,"/demo/DEV/username/project1/output'
programs: '",dir,"/demo/DEV/username/project1/programs'
data: !expr list(DEV = '", dir,"/demo/DEV/username/project1/data', PROD = '", dir, "/demo/PROD/project1/data')
output: '", dir, "/demo/DEV/username/project1/output'
programs: '", dir, "/demo/DEV/username/project1/programs'
envsetup_environ: !expr Sys.setenv(ENVSETUP_ENVIRON = 'DEV'); 'DEV'"
), file_conn)
), file_conn)
close(file_conn)
```

Expand Down

0 comments on commit facf9cd

Please sign in to comment.