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

Add argument for log_write to export log object as Rds #164

Merged
merged 15 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from 10 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
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# logrx 0.2.2

- Add `include_rds` argument to `axecute()` to export log as rds file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to upversion to 0.3.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parmsam-pfizer can you update NEWS to include this under 0.3.0 and resolve the merge conflict

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks!


# logrx 0.2.1

- non-function objects are no longer returned as functions by `get_used_functions` (#154)
Expand Down
8 changes: 7 additions & 1 deletion R/axecute.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#' @param log_path String. Path to log file
#' @param remove_log_object Boolean. Should the log object be removed after
#' writing the log file? Defaults to TRUE
#' @param include_rds Boolean. Option to export log object as Rds file.
#' Defaults to FALSE
#' @param quit_on_error Boolean. Should the session quit with status 1 on error?
#' Defaults to TRUE
#' @param to_report String vector. Objects to optionally report, may include as
Expand All @@ -34,6 +36,7 @@
axecute <- function(file, log_name = NA,
log_path = NA,
remove_log_object = TRUE,
include_rds = FALSE,
quit_on_error = TRUE,
to_report = c("messages", "output", "result")){

Expand All @@ -51,7 +54,10 @@ axecute <- function(file, log_name = NA,
any_errors <- get_log_element("errors")

# write log
log_write(file = file, remove_log_object = remove_log_object, to_report = to_report)
log_write(file = file,
remove_log_object = remove_log_object,
include_rds = include_rds,
to_report = to_report)

# if error, quit with status = 1 if not interactive
if(!interactive() & !is.null(any_errors) & quit_on_error) {
Expand Down
38 changes: 36 additions & 2 deletions R/log.R
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ log_cleanup <- function() {
#' to a log file
#'
#' @param file String. Path to file executed
#' @param include_rds Boolean. Option to export log object as Rds file.
#' Defaults to FALSE
#' @param remove_log_object Boolean. Should the log object be removed after
#' writing the log file? Defaults to TRUE
#' @param to_report String vector. Objects to optionally report; additional
Expand Down Expand Up @@ -177,6 +179,7 @@ log_cleanup <- function() {
#' log_write(file)
log_write <- function(file = NA,
remove_log_object = TRUE,
include_rds = FALSE,
to_report = c("messages", "output", "result")){
# Set end time and run time
set_log_element("end_time", strftime(Sys.time(), usetz = TRUE))
Expand Down Expand Up @@ -285,8 +288,39 @@ log_write <- function(file = NA,
write_log_element("log_path", "Log path: "))

writeLines(cleaned_log_vec,
con = file.path(get_log_element("log_path"),
get_log_element("log_name")))
con = file.path(get_log_element("log_path"),
get_log_element("log_name")))
if (include_rds){
rds_fields <- c(
"end_time", "start_time", "run_time", "user", "hash_sum",
"log_path", "log_name", "file_path", "file_name",
"unapproved_packages_functions", "errors", "warnings"
)
log_options <- as.list(getOption('log.rx'))
cleaned_log_list <- purrr::map2(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't 100% convinced myself, but I'm wondering if we would just return everything to the rds rather than NULL out sections. Those writing the log and those consuming these rds files might be different people with different use cases.

log_options,
names(log_options),
function(i, x){
if(x %in% c("messages", "output", "result")){
if(x %in% to_report){
return(i)
}
} else if(x %in% c(names(log_cleanup()), rds_fields)){
return(i)
}
}
)
cleaned_log_list$session_info <- session_info(info = "all")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have to do this because we capture the output of session_info and store it, rather than just storing the session info object. I think we can move the capture output to the write function so this will still print pretty, and then you won't need to get the session info again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Adjusted and updated the unit test.

saveRDS(cleaned_log_list,
file = file.path(
get_log_element("log_path"),
paste0(tools::file_path_sans_ext(
get_log_element("log_name")
),".Rds")
)
)
}

if (remove_log_object) {
log_remove()
}
Expand Down
4 changes: 4 additions & 0 deletions man/axecute.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions man/log_write.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 32 additions & 0 deletions tests/testthat/test-axecute.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,35 @@ test_that("to_report works to control log output elements", {
rm(flines, con, scriptPath, logDir)
log_remove()
})

test_that("include_rds works to output log as rds", {
options("log.rx" = NULL)
scriptPath <- tempfile()
logDir <- tempdir()
writeLines(
c("message('hello logrx')",
"cat('this is output')",
"data.frame(c(8, 6, 7, 5, 3, 0, 9))"),
con = scriptPath)

# check no log is currently written out
expect_warning(expect_error(file(file.path(logDir, "log_out_nested"), "r"), "cannot open the connection"))

axecute(scriptPath,
log_name = "log_out_nested",
log_path = logDir,
remove_log_object = FALSE,
include_rds = TRUE,
to_report = c("messages", "result"))
con <- file(file.path(logDir, "log_out_nested.Rds"), "r")
logRDS <- readRDS(file.path(logDir, "log_out_nested.Rds"))

expect_type(logRDS, "list")
expect_true("messages" %in% names(logRDS))
expect_true(all(is.na(logRDS$output)))
expect_true("result" %in% names(logRDS))
expect_true("start_time" %in% names(logRDS))

rm(con, scriptPath, logDir, logRDS)
log_remove()
})