From 002574cbac614c1f5f7901d239f319749d6d1c5a Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 08:05:48 -0700 Subject: [PATCH 01/19] rm logger test file --- tests/testthat/test-logger.R | 31 ------------------------------- 1 file changed, 31 deletions(-) delete mode 100644 tests/testthat/test-logger.R diff --git a/tests/testthat/test-logger.R b/tests/testthat/test-logger.R deleted file mode 100644 index bdc7c9e..0000000 --- a/tests/testthat/test-logger.R +++ /dev/null @@ -1,31 +0,0 @@ -context("logger") -withr::with_options(list(DataPackageR_verbose = TRUE),{ - test_that(".multilog_setup", { - expect_null(DataPackageR:::.multilog_setup(file.path(tempdir(), "test.log"))) - }) - test_that(".multilog_threshold", { - expect_null(DataPackageR:::.multilog_thresold(INFO, TRACE)) - }) - test_that(".multilog_info", { - expect_output(DataPackageR:::.multilog_info("message"), "INFO .* message") - expect_true(utils::file_test("-f", file.path(tempdir(), "test.log"))) - }) - test_that(".multilog_error", { - expect_output(DataPackageR:::.multilog_error("message"), "ERROR .* message") - }) - test_that(".multilog_trace", { - expect_silent(DataPackageR:::.multilog_trace("message")) - expect_true(length(grep(pattern = "TRACE", - readLines(file.path(tempdir(), - "test.log")))) > 0) - }) - test_that(".multilog_warn", { - expect_output(DataPackageR:::.multilog_warn("message"), "WARN") - }) - test_that(".multilog_debug", { - expect_silent(DataPackageR:::.multilog_debug("message")) - expect_true(length(grep(pattern = "DEBUG", - readLines(file.path(tempdir(), - "test.log")))) > 0) - }) -}) From 4758dc755fdbf3cc978345b3881661fe8626e6e4 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 08:08:20 -0700 Subject: [PATCH 02/19] rm logfile features from logger functions --- R/logger.R | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/R/logger.R b/R/logger.R index ea850a5..2594703 100644 --- a/R/logger.R +++ b/R/logger.R @@ -1,31 +1,24 @@ .multilog_info <- function(msg) { flog.info(msg, name = "console") - flog.info(msg, name = "logfile") } .multilog_trace <- function(msg) { flog.trace(msg, name = "console") - flog.trace(msg, name = "logfile") } .multilog_warn <- function(msg) { flog.warn(msg, name = "console") - flog.warn(msg, name = "logfile") } .multilog_debug <- function(msg) { flog.debug(msg, name = "console") - flog.debug(msg, name = "logfile") } .multilog_fatal <- function(msg) { flog.fatal(msg, name = "console") - flog.fatal(msg, name = "logfile") } .multilog_error <- function(msg) { flog.error(msg, name = "console") - flog.error(msg, name = "logfile") } .multilog_thresold <- function(console = INFO, logfile = TRACE) { flog.threshold(console, name = "console") - flog.threshold(logfile, name = "logfile") } select_console_appender <- function(){ @@ -38,17 +31,6 @@ select_console_appender <- function(){ } .multilog_setup <- function(LOGFILE = NULL) { - if (!is.null(LOGFILE)) { - if (file.exists(LOGFILE)){ - # initial newline to separate from previous run log entries - cat("\n", file = LOGFILE, append = TRUE) - } - flog.logger( - name = "logfile", - appender = appender.file(LOGFILE), - threshold = TRACE - ) - } flog.logger( name = "console", appender = select_console_appender(), From da4dc71632db7919b1ee8ca0f1421637e444e311 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 08:39:12 -0700 Subject: [PATCH 03/19] refactor to avoid ifelse problems after logging removal --- R/build.R | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/R/build.R b/R/build.R index 0643e6c..f6fca4b 100644 --- a/R/build.R +++ b/R/build.R @@ -88,10 +88,11 @@ package_build <- function(packageName = NULL, # Return success if we've processed everything success <- DataPackageR(arg = package_path, deps = deps) - ifelse(success, - .multilog_trace("DataPackageR succeeded"), + if (success){ + .multilog_trace("DataPackageR succeeded") + } else { .multilog_warn("DataPackageR failed") - ) + } .multilog_trace("Building documentation") local({ on.exit({ From 57d351b9964fe0f3aa15437f1f44b9644ea3a7a1 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 08:43:40 -0700 Subject: [PATCH 04/19] rm manual log setup from unit test file --- tests/testthat/test-edge-cases.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/testthat/test-edge-cases.R b/tests/testthat/test-edge-cases.R index c91ef76..5b8e643 100644 --- a/tests/testthat/test-edge-cases.R +++ b/tests/testthat/test-edge-cases.R @@ -225,8 +225,6 @@ test_that("local edge case block 8", { utils::package.skeleton("foo", path = td, environment = test_env, force = TRUE) td_foo <- file.path(td, 'foo') - DataPackageR:::.multilog_setup(file.path(td,"test.log")) - DataPackageR:::.multilog_thresold(INFO, TRACE) # data in digest changes while names do not suppressWarnings(expect_false({ DataPackageR:::.compare_digests( From b476839f30db21bc46e992d6eb947e850c47af73 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 08:52:58 -0700 Subject: [PATCH 05/19] rm unused logger functions --- R/logger.R | 6 ------ 1 file changed, 6 deletions(-) diff --git a/R/logger.R b/R/logger.R index 2594703..36676bb 100644 --- a/R/logger.R +++ b/R/logger.R @@ -1,15 +1,9 @@ -.multilog_info <- function(msg) { - flog.info(msg, name = "console") -} .multilog_trace <- function(msg) { flog.trace(msg, name = "console") } .multilog_warn <- function(msg) { flog.warn(msg, name = "console") } -.multilog_debug <- function(msg) { - flog.debug(msg, name = "console") -} .multilog_fatal <- function(msg) { flog.fatal(msg, name = "console") } From 71d659204d90b7d84231c816fe94446f3a4fb593 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 09:35:11 -0700 Subject: [PATCH 06/19] rm logfile-only traces --- R/build.R | 8 +------- R/digests.R | 21 ++------------------- R/load_save.R | 1 - R/logger.R | 3 --- R/processData.R | 39 --------------------------------------- R/prompt.R | 1 - 6 files changed, 3 insertions(+), 70 deletions(-) diff --git a/R/build.R b/R/build.R index f6fca4b..058299e 100644 --- a/R/build.R +++ b/R/build.R @@ -88,12 +88,7 @@ package_build <- function(packageName = NULL, # Return success if we've processed everything success <- DataPackageR(arg = package_path, deps = deps) - if (success){ - .multilog_trace("DataPackageR succeeded") - } else { - .multilog_warn("DataPackageR failed") - } - .multilog_trace("Building documentation") + if (! success) .multilog_warn("DataPackageR failed") local({ on.exit({ if (packageName %in% names(utils::sessionInfo()$otherPkgs)){ @@ -102,7 +97,6 @@ package_build <- function(packageName = NULL, }) roxygen2::roxygenize(package_path, clean = TRUE) }) - .multilog_trace("Building package") location <- pkgbuild::build(path = package_path, dest_path = dirname(package_path), vignettes = vignettes, diff --git a/R/digests.R b/R/digests.R index 86efa84..79658ed 100644 --- a/R/digests.R +++ b/R/digests.R @@ -21,8 +21,7 @@ .compare_digests <- function(old_digest, new_digest) { # Returns FALSE when any existing data has is changed, new data is added, or - # data is removed, else return TRUE. Use .multilog_trace for all changes since - # this is standard behavior during package re-build, and changes are already + # data is removed, else return TRUE. Changes are already # output to the console by .qualify_changes() old_digest[['DataVersion']] <- NULL @@ -33,23 +32,7 @@ removed <- setdiff(names(old_digest), names(new_digest)) common <- intersect(names(old_digest), names(new_digest)) changed <- common[new_digest[common] != old_digest[common]] - out <- TRUE - for(name in changed){ - .multilog_trace(paste(name, "has changed.")) - out <- FALSE - } - - for(name in removed){ - .multilog_trace(paste(name, "was removed.")) - out <- FALSE - } - - for(name in added){ - .multilog_trace(paste(name, "was added.")) - out <- FALSE - } - - return(out) + length(c(added, removed, changed)) == 0L } .combine_digests <- function(new, old) { diff --git a/R/load_save.R b/R/load_save.R index b99fba5..f445aa1 100644 --- a/R/load_save.R +++ b/R/load_save.R @@ -8,7 +8,6 @@ pkg_path = NULL) { DataVersion <- validate_DataVersion(DataVersion) .save_digest(new_data_digest, path = pkg_path) - .multilog_trace("Saving to data") # TODO get the names of each data object and save them separately. Provide a # function to load all. for (i in seq_along(object_names)) { diff --git a/R/logger.R b/R/logger.R index 36676bb..8098ca5 100644 --- a/R/logger.R +++ b/R/logger.R @@ -1,6 +1,3 @@ -.multilog_trace <- function(msg) { - flog.trace(msg, name = "console") -} .multilog_warn <- function(msg) { flog.warn(msg, name = "console") } diff --git a/R/processData.R b/R/processData.R index 5d26f04..c070566 100644 --- a/R/processData.R +++ b/R/processData.R @@ -50,10 +50,8 @@ DataPackageR <- function(arg = NULL, deps = TRUE) { LOGFILE <- file.path(logpath, "processing.log") .multilog_setup(LOGFILE) .multilog_thresold(console = INFO, logfile = TRACE) - .multilog_trace(paste0("Logging to ", LOGFILE)) # validate package validate_package_skeleton(pkg_dir) - .multilog_trace("Processing data") # validate datapackager.yml ymlconf <- validate_yml(pkg_dir) # get vector of R and Rmd files from validated YAML @@ -90,10 +88,6 @@ DataPackageR <- function(arg = NULL, deps = TRUE) { # assign ENVS into dataenv. # provide functions in the package to read from it (if deps = TRUE) if (deps) assign(x = "ENVS", value = ENVS, dataenv) - .multilog_trace(paste0( - "Processing ", i, " of ", - length(r_files), ": ", r_files[i] - )) # config file goes in the root render the r and rmd files ## First we spin then render if it's an R file flag <- FALSE @@ -137,11 +131,6 @@ DataPackageR <- function(arg = NULL, deps = TRUE) { object_tally <- object_tally | objects_to_keep %in% object_names already_built <- unique(c(already_built, objects_to_keep[objects_to_keep %in% object_names])) - .multilog_trace(paste0( - sum(objects_to_keep %in% object_names), - " data set(s) created by ", - basename(r_files[i]) - )) .done(paste0( sum(objects_to_keep %in% object_names), " data set(s) created by ", @@ -191,7 +180,6 @@ DataPackageR <- function(arg = NULL, deps = TRUE) { do_doc(pkg_dir, dataenv) # copy html files to vignettes .ppfiles_mkvignettes(dir = pkg_dir) - .multilog_trace("Done") return(TRUE) } @@ -246,7 +234,6 @@ validate_yml <- function(pkg_dir){ .multilog_fatal("YAML is missing files: and objects: entries") stop("exiting", call. = FALSE) } - .multilog_trace("Reading yaml configuration") # files that have enable: TRUE stopifnot("configuration" %in% names(ymlconf)) stopifnot("files" %in% names(ymlconf[["configuration"]])) @@ -282,7 +269,6 @@ validate_yml <- function(pkg_dir){ stop(err_msg, call. = FALSE) } } - .multilog_trace(paste0("Found ", r_files)) return(ymlconf) } @@ -397,11 +383,6 @@ do_digests <- function(pkg_dir, dataenv) { } if (same_digests && check_new_DataVersion == "equal") { can_write <- TRUE - .multilog_trace(paste0( - "Processed data sets match ", - "existing data sets at version ", - new_data_digest[["DataVersion"]] - )) } else if ((! same_digests) && check_new_DataVersion == "equal") { updated_version <- .increment_data_version( pkg_desc, @@ -417,26 +398,13 @@ do_digests <- function(pkg_dir, dataenv) { pkg_desc <- updated_version$pkg_description new_data_digest <- updated_version$new_data_digest can_write <- TRUE - .multilog_trace(paste0( - "Data has been updated and DataVersion ", - "string incremented automatically to ", - new_data_digest[["DataVersion"]] - )) } else if (same_digests && check_new_DataVersion == "higher") { # edge case that shouldn't happen # but we test for it in the test suite can_write <- TRUE - .multilog_trace(paste0( - "Data hasn't changed but the ", - "DataVersion has been bumped." - )) } else if (check_new_DataVersion == "lower" && same_digests) { # edge case that shouldn't happen but # we test for it in the test suite. - .multilog_trace(paste0( - "New DataVersion is less than ", - "old but data are unchanged" - )) new_data_digest <- old_data_digest pkg_desc$set('DataVersion', validate_DataVersion(new_data_digest[["DataVersion"]]) @@ -521,12 +489,6 @@ do_doc <- function(pkg_dir, dataenv) { writeLines(Reduce(c, doc_parsed), file.path(pkg_dir, "R", paste0(pkg_name, ".R")) ) - .multilog_trace( - paste0( - "Copied documentation to ", - file.path(pkg_dir, "R", paste0(pkg_name, ".R")) - ) - ) # TODO test that we have documented # everything successfully and that all files # have been parsed successfully @@ -789,7 +751,6 @@ document <- function(path = ".", install = FALSE, ...) { to = file.path(path, "R", docfile), overwrite = TRUE ) - .multilog_trace("Rebuilding data package documentation.") local({ on.exit({ if (basename(path) %in% names(utils::sessionInfo()$otherPkgs)){ diff --git a/R/prompt.R b/R/prompt.R index b504b74..9347efc 100644 --- a/R/prompt.R +++ b/R/prompt.R @@ -53,7 +53,6 @@ .newsfile <- function() { newsfile <- file.path(usethis::proj_get(), "NEWS.md") if (!file.exists(newsfile)) { - .multilog_trace("NEWS.md file not found, creating!") file.create(newsfile) } return(newsfile) From 78a8143ebaaabfc5bd6a9f472c87c5f96b207686 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 09:40:00 -0700 Subject: [PATCH 07/19] rm .multilog_thresold() [sic] --- R/logger.R | 4 ---- R/processData.R | 1 - 2 files changed, 5 deletions(-) diff --git a/R/logger.R b/R/logger.R index 8098ca5..bfb003f 100644 --- a/R/logger.R +++ b/R/logger.R @@ -8,10 +8,6 @@ flog.error(msg, name = "console") } -.multilog_thresold <- function(console = INFO, logfile = TRACE) { - flog.threshold(console, name = "console") -} - select_console_appender <- function(){ if (getOption('DataPackageR_verbose', TRUE)){ appender.console() diff --git a/R/processData.R b/R/processData.R index c070566..2b7ea85 100644 --- a/R/processData.R +++ b/R/processData.R @@ -49,7 +49,6 @@ DataPackageR <- function(arg = NULL, deps = TRUE) { # open a log file LOGFILE <- file.path(logpath, "processing.log") .multilog_setup(LOGFILE) - .multilog_thresold(console = INFO, logfile = TRACE) # validate package validate_package_skeleton(pkg_dir) # validate datapackager.yml From 059472be866f6a1bfaefaaf68629f4d09010333f Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 09:41:54 -0700 Subject: [PATCH 08/19] rm multilog_error --- R/logger.R | 3 --- R/processData.R | 1 - 2 files changed, 4 deletions(-) diff --git a/R/logger.R b/R/logger.R index bfb003f..bac7cb1 100644 --- a/R/logger.R +++ b/R/logger.R @@ -4,9 +4,6 @@ .multilog_fatal <- function(msg) { flog.fatal(msg, name = "console") } -.multilog_error <- function(msg) { - flog.error(msg, name = "console") -} select_console_appender <- function(){ if (getOption('DataPackageR_verbose', TRUE)){ diff --git a/R/processData.R b/R/processData.R index 2b7ea85..41a4f6b 100644 --- a/R/processData.R +++ b/R/processData.R @@ -2,7 +2,6 @@ # catch an error if it doesn't exist, otherwise return normalized path # important for handling relative paths in a rmarkdown::render() context if (! dir.exists(x)){ - .multilog_error(paste0("render_root = ", x, " doesn't exist")) stop(paste0("render_root = ", x, " doesn't exist")) } normalizePath(x, winslash = "/") From 36918a06cdc6b46f82b96cf119329a0c7964565e Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 09:51:00 -0700 Subject: [PATCH 09/19] rm multilog_warn --- R/build.R | 7 +++---- R/logger.R | 3 --- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/R/build.R b/R/build.R index 058299e..b4825db 100644 --- a/R/build.R +++ b/R/build.R @@ -85,10 +85,9 @@ package_build <- function(packageName = NULL, # Check that directory name matches package name validate_pkg_name(package_path) - # Return success if we've processed everything - success <- - DataPackageR(arg = package_path, deps = deps) - if (! success) .multilog_warn("DataPackageR failed") + # Process everything + DataPackageR(arg = package_path, deps = deps) + local({ on.exit({ if (packageName %in% names(utils::sessionInfo()$otherPkgs)){ diff --git a/R/logger.R b/R/logger.R index bac7cb1..0d8033d 100644 --- a/R/logger.R +++ b/R/logger.R @@ -1,6 +1,3 @@ -.multilog_warn <- function(msg) { - flog.warn(msg, name = "console") -} .multilog_fatal <- function(msg) { flog.fatal(msg, name = "console") } From 05035534e20b370984275feb834eef8478d14507 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 10:14:14 -0700 Subject: [PATCH 10/19] rm multilog_fatal --- R/logger.R | 4 --- R/processData.R | 39 ++++++++------------------ R/yamlR.R | 9 ++---- tests/testthat/test-data-name-change.R | 5 +++- 4 files changed, 18 insertions(+), 39 deletions(-) diff --git a/R/logger.R b/R/logger.R index 0d8033d..5b20260 100644 --- a/R/logger.R +++ b/R/logger.R @@ -1,7 +1,3 @@ -.multilog_fatal <- function(msg) { - flog.fatal(msg, name = "console") -} - select_console_appender <- function(){ if (getOption('DataPackageR_verbose', TRUE)){ appender.console() diff --git a/R/processData.R b/R/processData.R index 41a4f6b..2def4ef 100644 --- a/R/processData.R +++ b/R/processData.R @@ -219,18 +219,15 @@ validate_yml <- function(pkg_dir){ full.names = TRUE ) if (length(ymlfile) == 0) { - .multilog_fatal(paste0("Yaml configuration file not found at ", pkg_dir)) - stop("exiting", call. = FALSE) + stop(paste("Yaml configuration file not found at", pkg_dir)) } ymlconf <- read_yaml(ymlfile) # test that the structure of the yaml file is correct! if (!"configuration" %in% names(ymlconf)) { - .multilog_fatal("YAML is missing 'configuration:' entry") - stop("exiting", call. = FALSE) + stop("YAML is missing 'configuration:' entry") } if (!all(c("files", "objects") %in% names(ymlconf$configuration))) { - .multilog_fatal("YAML is missing files: and objects: entries") - stop("exiting", call. = FALSE) + stop("YAML is missing files: and objects: entries") } # files that have enable: TRUE stopifnot("configuration" %in% names(ymlconf)) @@ -247,24 +244,18 @@ validate_yml <- function(pkg_dir){ render_root <- .get_render_root(ymlconf) .validate_render_root(render_root) if (length(get_yml_objects(ymlconf)) == 0) { - .multilog_fatal("You must specify at least one data object.") - stop("exiting", call. = FALSE) + stop("You must specify at least one data object.") } r_files <- get_yml_r_files(ymlconf) if (length(r_files) == 0) { - .multilog_fatal("No files enabled for processing!") - stop("error", call. = FALSE) + stop("No files enabled for processing!") } if (any(duplicated(r_files))){ - err_msg <- "Duplicate R files specified in YAML." - .multilog_fatal(err_msg) - stop(err_msg, call. = FALSE) + stop("Duplicate R files specified in YAML.") } for (file in r_files){ if (! file.exists(file.path(pkg_dir, 'data-raw', file))){ - err_msg <- paste("Missing R file specified in YAML:", file) - .multilog_fatal(err_msg) - stop(err_msg, call. = FALSE) + stop(paste("Missing R file specified in YAML:", file)) } } return(ymlconf) @@ -283,9 +274,7 @@ validate_package_skeleton <- function(pkg_dir){ dirs <- file.path(pkg_dir, c("R", "inst", "data", "data-raw")) for (dir in dirs){ if (! utils::file_test(dir, op = "-d")){ - err_msg <- paste("Missing required subdirectory", dir) - .multilog_fatal(err_msg) - stop(err_msg) + stop(paste("Missing required subdirectory", dir)) } } # check we can read a DESCRIPTION file @@ -303,12 +292,10 @@ validate_description <- function(pkg_dir){ d <- desc::desc(pkg_dir) dv <- d$get('DataVersion') if (is.na(dv)) { - err_msg <- paste0( + stop(paste( "DESCRIPTION file must have a DataVersion", - " line. i.e. DataVersion: 0.2.0" - ) - .multilog_fatal(err_msg) - stop(err_msg, call. = FALSE) + "line. i.e. DataVersion: 0.2.0" + )) } validate_DataVersion(dv) d @@ -375,9 +362,7 @@ do_digests <- function(pkg_dir, dataenv) { same_digests <- .compare_digests(old_data_digest, new_data_digest) if ((! same_digests) && check_new_DataVersion == "higher"){ # not sure how this would actually happen - err_msg <- 'Digest(s) differ but DataVersion had already been incremented' - .multilog_fatal(err_msg) - stop(err_msg, call. = FALSE) + stop('Digest(s) differ but DataVersion had already been incremented') } if (same_digests && check_new_DataVersion == "equal") { can_write <- TRUE diff --git a/R/yamlR.R b/R/yamlR.R index 06640f5..f4b890b 100644 --- a/R/yamlR.R +++ b/R/yamlR.R @@ -262,11 +262,7 @@ construct_yml_config <- function(code = NULL, data = NULL, render_root = NULL) { silent = TRUE ) if (inherits(render_root, "try-error")) { - .multilog_fatal(paste0( - dirname(render_root), - " doesn't exist!" - )) - stop("error", call. = FALSE) + stop(paste(dirname(render_root), "doesn't exist!")) } yml[["configuration"]]$render_root <- render_root } @@ -281,7 +277,6 @@ construct_yml_config <- function(code = NULL, data = NULL, render_root = NULL) { } else if (length(x$configuration$render_root) != 0) { return(x$configuration$render_root) } else { - .multilog_fatal("render_root is not set in yaml") - stop("error", call. = FALSE) + stop("render_root is not set in yaml") } } diff --git a/tests/testthat/test-data-name-change.R b/tests/testthat/test-data-name-change.R index f3d19e6..136a484 100644 --- a/tests/testthat/test-data-name-change.R +++ b/tests/testthat/test-data-name-change.R @@ -39,7 +39,10 @@ test_that("data object can be renamed", { datapackage_skeleton(pname, tempdir(), force = TRUE) addData("mtcars", pname) expect_no_error(changeName("mtcars", "mtcars2", pname)) - expect_error(removeName("mtcars2", "mtcars.R", pname), "exiting") + expect_error( + removeName("mtcars2", "mtcars.R", pname), + "You must specify at least one data object." + ) ## test change when two objects are present pname <- "nameChangeTest2" From a9ac9b299855b4559d30807eafc747b4843feb1a Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 10:25:46 -0700 Subject: [PATCH 11/19] rm direct calls to flog functions --- R/build.R | 28 +++++++++++----------------- R/processData.R | 4 +--- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/R/build.R b/R/build.R index b4825db..cecb283 100644 --- a/R/build.R +++ b/R/build.R @@ -48,7 +48,6 @@ package_build <- function(packageName = NULL, install = FALSE, ...) { .multilog_setup(LOGFILE = NULL) - # flog.appender(appender.console()) if (is.null(packageName)) { packageName <- "." # use normalizePath @@ -56,16 +55,12 @@ package_build <- function(packageName = NULL, packageName <- basename(package_path) # Is this a package root? if (!is_r_package$find_file() == package_path) { - flog.fatal(paste0(package_path, - " is not an R package root directory"), - name = "console") - stop("exiting", call. = FALSE) + stop(paste(package_path, "is not an R package root directory")) } } else { package_path <- normalizePath(packageName, winslash = "/") if (!file.exists(package_path)) { - flog.fatal(paste0("Non existent package ", packageName), name = "console") - stop("exiting", call. = FALSE) + stop(paste("Non existent package", packageName)) } packageName <- basename(package_path) } @@ -73,12 +68,13 @@ package_build <- function(packageName = NULL, # subdirectory tryCatch({is_r_package$find_file(path = package_path)}, error = function(cond){ - flog.fatal(paste0( - package_path, - " is not a valid R package directory beneath ", - getwd() - ), name = "console") - stop("exiting", call. = FALSE) + stop( + paste( + package_path, + "is not a valid R package directory beneath", + getwd() + ) + ) } ) @@ -136,10 +132,8 @@ validate_pkg_name <- function(package_path){ )$get("Package") path_pkg_name <- basename(package_path) if (desc_pkg_name != path_pkg_name){ - err_msg <- paste("Data package name in DESCRIPTION does not match", - "name of the data package directory") - flog.fatal(err_msg, name = "console") - stop(err_msg, call. = FALSE) + stop(paste("Data package name in DESCRIPTION does not match", + "name of the data package directory")) } desc_pkg_name } diff --git a/R/processData.R b/R/processData.R index 2def4ef..b08048e 100644 --- a/R/processData.R +++ b/R/processData.R @@ -237,9 +237,7 @@ validate_yml <- function(pkg_dir){ # object with same name as package causes problems with # overwriting documentation files if (basename(pkg_dir) %in% ymlconf$configuration$objects){ - err_msg <- "Data object not allowed to have same name as data package" - flog.fatal(err_msg, name = "console") - stop(err_msg, call. = FALSE) + stop("Data object not allowed to have same name as data package") } render_root <- .get_render_root(ymlconf) .validate_render_root(render_root) From 42591af15d636036a8c2045c68ee77dc960a6d14 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 10:35:08 -0700 Subject: [PATCH 12/19] rm call. = FALSE --- R/skeleton.R | 6 +++--- R/use.R | 6 +++--- R/yamlR.R | 8 ++------ 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/R/skeleton.R b/R/skeleton.R index b4ee163..d2e411b 100644 --- a/R/skeleton.R +++ b/R/skeleton.R @@ -56,13 +56,13 @@ datapackage_skeleton <- options(usethis.quiet = TRUE) } if (is.null(name)) { - stop("Must supply a package name", call. = FALSE) + stop("Must supply a package name") } # if (length(r_object_names) == 0) { - # stop("You must specify r_object_names", call. = FALSE) + # stop("You must specify r_object_names") # } # if (length(code_files) == 0) { - # stop("You must specify code_files", call. = FALSE) + # stop("You must specify code_files") # } if (force) { unlink(file.path(path, name), recursive = TRUE, force = TRUE) diff --git a/R/use.R b/R/use.R index 4afd569..8cf424d 100644 --- a/R/use.R +++ b/R/use.R @@ -31,7 +31,7 @@ use_raw_dataset <- function(path = NULL, ignore = FALSE) { } proj_path <- usethis::proj_get() if (!utils::file_test("-d", file.path(proj_path, "inst", "extdata"))) { - stop(paste0("inst/extdata doesn't exist in ", proj_path), call. = FALSE) + stop(paste0("inst/extdata doesn't exist in ", proj_path)) } raw_file <- normalizePath(path) if (utils::file_test("-f", raw_file)) { @@ -99,7 +99,7 @@ use_processing_script <- function(file = NULL, title = NULL, author = NULL, over } proj_path <- usethis::proj_get() if (!utils::file_test("-d", file.path(proj_path, "data-raw"))) { - stop(paste0("data-raw doesn't exist in ", proj_path), call. = FALSE) + stop(paste0("data-raw doesn't exist in ", proj_path)) } #check if the given file or directory already exists if (utils::file_test("-f",file.path(proj_path,"data-raw",file))|utils::file_test("-d",file.path(proj_path,"data-raw",file))) { #nolint @@ -341,7 +341,7 @@ use_data_object <- function(object_name = NULL) { .validate_front_matter <- function(front_matter) { front_matter <- .trim_trailing_ws(front_matter) if (grepl(":$", front_matter)) { - stop("Invalid YAML front matter (ends with ':')", call. = FALSE) + stop("Invalid YAML front matter (ends with ':')") } } diff --git a/R/yamlR.R b/R/yamlR.R index f4b890b..0420031 100644 --- a/R/yamlR.R +++ b/R/yamlR.R @@ -35,10 +35,7 @@ yml_find <- function(path) { path <- normalizePath(path, winslash = "/") config_yml <- is_r_package$find_file("datapackager.yml", path = path) if (!file.exists(config_yml)) { - stop("Can't find a datapackager.yml config at ", - dirname(config_yml), - call. = FALSE - ) + stop("Can't find a datapackager.yml config at ", dirname(config_yml)) } config <- yaml::yaml.load_file(config_yml) attr(config, "path") <- config_yml @@ -182,8 +179,7 @@ yml_write <- function(config, path = NULL) { paste0( "config must be a datapackager.yml configuration", " in r object representation, as ready by yml_find()" - ), - call. = FALSE + ) ) } if (is.null(path)) { From c31f27e46f90ab00a7bd7ac7246d01f59a98f1c9 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 10:43:52 -0700 Subject: [PATCH 13/19] rm logging from documentation --- vignettes/Using_DataPackageR.Rmd | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/vignettes/Using_DataPackageR.Rmd b/vignettes/Using_DataPackageR.Rmd index 126fe8f..68f73a0 100644 --- a/vignettes/Using_DataPackageR.Rmd +++ b/vignettes/Using_DataPackageR.Rmd @@ -169,11 +169,7 @@ A description of your changes to the package [The rest of the file] ``` -### Logging the build process. - -DataPackageR uses the `futile.logger` package to log progress. - -If there are errors in the processing, the script will notify you via logging to console and to `/private/tmp/Test/inst/extdata/Logfiles/processing.log`. Errors should be corrected and the build repeated. +### The build. If everything goes smoothly, you will have a new package built in the parent directory. From ef63cec33450c05e9e41d7c15f2905360919b329 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 10:48:13 -0700 Subject: [PATCH 14/19] rm logfile from unrelated unit test --- tests/testthat/test-project-path.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-project-path.R b/tests/testthat/test-project-path.R index 655cf2f..d205cce 100644 --- a/tests/testthat/test-project-path.R +++ b/tests/testthat/test-project-path.R @@ -23,7 +23,7 @@ test_that("project_data_path works with file arguments", { expect_equal(project_data_path("cars_over_20.rda"), expected = file.path(usethis::proj_get(), "data", "cars_over_20.rda")) # nolint }) test_that("project_extdata_path works with file arguments", { - expect_equal(project_extdata_path("Logfiles/processing.log"), expected = file.path(usethis::proj_get(), "inst", "extdata", "Logfiles", "processing.log")) # nolint + expect_equal(project_extdata_path("Logfiles/subsetCars.html"), expected = file.path(usethis::proj_get(), "inst", "extdata", "Logfiles", "subsetCars.html")) # nolint }) unlink(file.path(tempdir(), "subsetCars"), recursive = TRUE, From 5529d4ffe0b1a059c8745c1c649f440a7e0e4397 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 10:49:09 -0700 Subject: [PATCH 15/19] rm log argument from package_build() --- R/build.R | 3 --- R/processData.R | 3 --- man/package_build.Rd | 3 --- 3 files changed, 9 deletions(-) diff --git a/R/build.R b/R/build.R index cecb283..ef09fd5 100644 --- a/R/build.R +++ b/R/build.R @@ -6,7 +6,6 @@ #' #' @param packageName \code{character} path to package source directory. Defaults to the current path when NULL. #' @param vignettes \code{logical} specify whether to build vignettes. Default FALSE. -#' @param log log level \code{INFO,WARN,DEBUG,FATAL} #' @param deps \code{logical} should we pass data objects into subsequent scripts? Default TRUE #' @param install \code{logical} automatically install and load the package after building. Default FALSE #' @param ... additional arguments passed to \code{install.packages} when \code{install=TRUE}. @@ -43,11 +42,9 @@ #' } package_build <- function(packageName = NULL, vignettes = FALSE, - log = INFO, deps = TRUE, install = FALSE, ...) { - .multilog_setup(LOGFILE = NULL) if (is.null(packageName)) { packageName <- "." # use normalizePath diff --git a/R/processData.R b/R/processData.R index b08048e..0674f5b 100644 --- a/R/processData.R +++ b/R/processData.R @@ -45,9 +45,6 @@ DataPackageR <- function(arg = NULL, deps = TRUE) { logpath <- file.path(pkg_dir, "inst", "extdata", "Logfiles") dir.create(logpath, recursive = TRUE, showWarnings = FALSE) - # open a log file - LOGFILE <- file.path(logpath, "processing.log") - .multilog_setup(LOGFILE) # validate package validate_package_skeleton(pkg_dir) # validate datapackager.yml diff --git a/man/package_build.Rd b/man/package_build.Rd index f52f63c..2802708 100644 --- a/man/package_build.Rd +++ b/man/package_build.Rd @@ -7,7 +7,6 @@ package_build( packageName = NULL, vignettes = FALSE, - log = INFO, deps = TRUE, install = FALSE, ... @@ -18,8 +17,6 @@ package_build( \item{vignettes}{\code{logical} specify whether to build vignettes. Default FALSE.} -\item{log}{log level \code{INFO,WARN,DEBUG,FATAL}} - \item{deps}{\code{logical} should we pass data objects into subsequent scripts? Default TRUE} \item{install}{\code{logical} automatically install and load the package after building. Default FALSE} From 501d07bed54d20003b8a2ed985ba420a167d0e0d Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 10:52:35 -0700 Subject: [PATCH 16/19] rm logger.R --- R/logger.R | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 R/logger.R diff --git a/R/logger.R b/R/logger.R deleted file mode 100644 index 5b20260..0000000 --- a/R/logger.R +++ /dev/null @@ -1,16 +0,0 @@ -select_console_appender <- function(){ - if (getOption('DataPackageR_verbose', TRUE)){ - appender.console() - } else { - # quiet console appender - function(line) { } - } -} - -.multilog_setup <- function(LOGFILE = NULL) { - flog.logger( - name = "console", - appender = select_console_appender(), - threshold = INFO - ) -} From 9d94d4b0d19343a09b5d9837c4aa2ec2f66629bb Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 11:00:45 -0700 Subject: [PATCH 17/19] drop dependency on futile.logger package --- DESCRIPTION | 1 - NAMESPACE | 14 -------------- R/build.R | 1 - 3 files changed, 16 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index ecefe63..05efaed 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -41,7 +41,6 @@ Imports: cli, desc, digest, - futile.logger, knitr, pkgbuild, pkgload, diff --git a/NAMESPACE b/NAMESPACE index 65252e9..9b3f767 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -28,20 +28,6 @@ export(yml_remove_files) export(yml_remove_objects) export(yml_write) importFrom(desc,desc) -importFrom(futile.logger,INFO) -importFrom(futile.logger,TRACE) -importFrom(futile.logger,appender.console) -importFrom(futile.logger,appender.file) -importFrom(futile.logger,appender.tee) -importFrom(futile.logger,flog.appender) -importFrom(futile.logger,flog.debug) -importFrom(futile.logger,flog.error) -importFrom(futile.logger,flog.fatal) -importFrom(futile.logger,flog.info) -importFrom(futile.logger,flog.logger) -importFrom(futile.logger,flog.threshold) -importFrom(futile.logger,flog.trace) -importFrom(futile.logger,flog.warn) importFrom(knitr,knit) importFrom(knitr,spin) importFrom(rmarkdown,pandoc_available) diff --git a/R/build.R b/R/build.R index ef09fd5..4cf37d5 100644 --- a/R/build.R +++ b/R/build.R @@ -14,7 +14,6 @@ #' @importFrom rprojroot is_r_package #' @importFrom rmarkdown pandoc_available #' @importFrom yaml read_yaml -#' @importFrom futile.logger flog.logger flog.trace appender.file flog.debug flog.info flog.warn flog.error flog.fatal flog.appender flog.threshold INFO TRACE appender.console appender.tee #' @importFrom knitr knit spin #' @details Note that if \code{package_build} returns an error when rendering an \code{.Rmd} #' internally, but that same \code{.Rmd} can be run successfully manually using \code{rmarkdown::render}, From 7b94edaacf17cd0b22603b6f9922cca62c1b5424 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 11:42:55 -0700 Subject: [PATCH 18/19] NEWS update --- NEWS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/NEWS.md b/NEWS.md index ea32cfd..e7fff9f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,17 @@ # DataPackageR (development version) +## Significant user-facing changes +* Remove functionality for logging to a logfile (#163) +* Remove 'log' argument from package_build() (#163) + +## Unchanged +* User still sees same messages, warnings, and errors on the console +* Changes to data objects still automatically added to the data package NEWS.md +* For now, rendered output files still written to inst/extdata/Logfiles + +## Maintenance +* Drop dependency on futile.logger package, which has not been updated since 2016. + # DataPackageR 0.16.1 ## Minor user-facing improvements From 924b833189767d1ed1742f8fe10e042cc869597f Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 12 Oct 2024 11:48:51 -0700 Subject: [PATCH 19/19] update WORDLIST --- inst/WORDLIST | 2 ++ 1 file changed, 2 insertions(+) diff --git a/inst/WORDLIST b/inst/WORDLIST index 51065b4..bd7830a 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -10,6 +10,7 @@ FlowRepository Hadley ImmPort ImmuneSpace +Logfiles ORCID Pre Preprint @@ -41,6 +42,7 @@ gitignore https incrementing loc +logfile md mtcars mydata