From 518e2ac86fe3b74128199140f528c1a19e2a33a8 Mon Sep 17 00:00:00 2001 From: "zxBIB Schreyer,Daniel (TMCP TM&BM P) EXTERNAL" Date: Wed, 11 Dec 2024 10:11:31 +0100 Subject: [PATCH 1/7] resolved issue #22. Yes/No will not be converted to booleans --- CHANGELOG.md | 6 ++++++ DESCRIPTION | 2 +- R/read_params.R | 26 +++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 990f503..b956aed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## v0.6.1 + +### Fixes + +- Improved the read_params function by enforcing the params.yaml to be read as is. "Yes", "No" will not be converted to booleans anymore. Related to issue #22 + ## v0.6 ### New Features diff --git a/DESCRIPTION b/DESCRIPTION index 1cf4126..7da0a39 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: dso Type: Package Title: dso R companion package -Version: 0.6 +Version: 0.6.1 Author: Daniel Schreyer, Gregor Sturm, Thomas Schwarzl diff --git a/R/read_params.R b/R/read_params.R index 653290f..ab92794 100644 --- a/R/read_params.R +++ b/R/read_params.R @@ -12,6 +12,7 @@ #' #' @return parameters as list of list as `dsoParams` or conventional list when `return_list` is set. #' @importFrom yaml read_yaml +#' @importFrom purrr modify_tree #' @export read_params <- function(stage_path = NULL, return_list = FALSE) { if (!is.logical(return_list)) { @@ -73,7 +74,30 @@ read_params <- function(stage_path = NULL, return_list = FALSE) { } ) - yaml <- read_yaml(tmp_config_file) + yaml <- read_yaml(tmp_config_file, + handlers = list( + "bool#yes" = \(x) { + attr(x, "yaml_bool") <- TRUE + x + }, + "bool#no" = \(x) { + attr(x, "yaml_bool") <- FALSE + x + } + ) + ) |> + purrr::modify_tree(leaf = \(x) { + if (is.character(x) && is.logical(attr(x, "yaml_bool"))) { + if (x %in% c("true", "false")) { + return(attr(x, "yaml_bool")) + } else { + attr(x, "yaml_bool") <- NULL + return(x) + } + } else { + return(x) + } + }) unlink(tmp_config_file) if (return_list) { From 285c5635a4d6bf6bf2be1b6cdf011bf182128407 Mon Sep 17 00:00:00 2001 From: DSchreyer Date: Wed, 11 Dec 2024 09:15:59 +0000 Subject: [PATCH 2/7] pre-commit autofixes --- NAMESPACE | 1 + 1 file changed, 1 insertion(+) diff --git a/NAMESPACE b/NAMESPACE index 3b06293..07261a9 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -22,6 +22,7 @@ importFrom(glue,glue) importFrom(here,here) importFrom(here,i_am) importFrom(methods,show) +importFrom(purrr,modify_tree) importFrom(rlang,caller_env) importFrom(rstudioapi,viewer) importFrom(stringr,coll) From 44dd454e44414ab6e35ffadc8327713cc2d9f6a7 Mon Sep 17 00:00:00 2001 From: "zxBIB Schreyer,Daniel (TMCP TM&BM P) EXTERNAL" Date: Mon, 13 Jan 2025 12:04:13 +0100 Subject: [PATCH 3/7] introduced read_safe_yaml function for safely reading in yes and no --- DESCRIPTION | 3 ++- NAMESPACE | 1 + R/read_params.R | 62 ++++++++++++++++++++++++++++--------------------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 440fd33..c461df5 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -19,7 +19,8 @@ Imports: stringr, methods, rlang, - rstudioapi (>= 0.11) + rstudioapi (>= 0.11), + purrr RoxygenNote: 7.3.2 Suggests: testthat (>= 3.0.0) diff --git a/NAMESPACE b/NAMESPACE index 07261a9..2fdd9d9 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -10,6 +10,7 @@ export(dso_repro_stage_addin) export(dso_repro_stage_w_dependencies_addin) export(init) export(read_params) +export(read_safe_yaml) export(reload) export(repro) export(safe_get) diff --git a/R/read_params.R b/R/read_params.R index ab92794..d71484d 100644 --- a/R/read_params.R +++ b/R/read_params.R @@ -11,8 +11,6 @@ #' @param return_list returns a list if TRUE, by default it return `dsoParams` class which is a list with secure access #' #' @return parameters as list of list as `dsoParams` or conventional list when `return_list` is set. -#' @importFrom yaml read_yaml -#' @importFrom purrr modify_tree #' @export read_params <- function(stage_path = NULL, return_list = FALSE) { if (!is.logical(return_list)) { @@ -74,30 +72,7 @@ read_params <- function(stage_path = NULL, return_list = FALSE) { } ) - yaml <- read_yaml(tmp_config_file, - handlers = list( - "bool#yes" = \(x) { - attr(x, "yaml_bool") <- TRUE - x - }, - "bool#no" = \(x) { - attr(x, "yaml_bool") <- FALSE - x - } - ) - ) |> - purrr::modify_tree(leaf = \(x) { - if (is.character(x) && is.logical(attr(x, "yaml_bool"))) { - if (x %in% c("true", "false")) { - return(attr(x, "yaml_bool")) - } else { - attr(x, "yaml_bool") <- NULL - return(x) - } - } else { - return(x) - } - }) + yaml <- read_clean_yaml(tmp_config_file) unlink(tmp_config_file) if (return_list) { @@ -149,3 +124,38 @@ set_stage <- function(stage_path) { stage_here <- function(...) { file.path(config_env$stage_dir, ...) } + +#' @title read_safe_yaml +#' @description +#' Safely reads in yaml files +#' @export +#' @return a list +#' @importFrom yaml read_yaml +#' @importFrom purrr modify_tree +#' @export +read_safe_yaml <- function(params_file) { + yaml <- read_yaml(params_file, + handlers = list( + "bool#yes" = \(x) { + attr(x, "yaml_bool") <- TRUE + x + }, + "bool#no" = \(x) { + attr(x, "yaml_bool") <- FALSE + x + } + ) + ) |> + purrr::modify_tree(leaf = \(x) { + if (is.character(x) && is.logical(attr(x, "yaml_bool"))) { + if (x %in% c("true", "false")) { + return(attr(x, "yaml_bool")) + } else { + attr(x, "yaml_bool") <- NULL + return(x) + } + } else { + return(x) + } + }) +} From 943112e13f92a64a7d0cef24bd3af4571e1ea33b Mon Sep 17 00:00:00 2001 From: "zxBIB Schreyer,Daniel (TMCP TM&BM P) EXTERNAL" Date: Mon, 13 Jan 2025 12:08:01 +0100 Subject: [PATCH 4/7] added testthat for safely reading yaml --- tests/testthat/test-read_params.R | 51 +++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/testthat/test-read_params.R diff --git a/tests/testthat/test-read_params.R b/tests/testthat/test-read_params.R new file mode 100644 index 0000000..17ff5a9 --- /dev/null +++ b/tests/testthat/test-read_params.R @@ -0,0 +1,51 @@ +library(yaml) +test_that("read_params: yes and no parameters are correctly loaded", { + ### Create Test Data + # Create a temporary YAML file + temp_file <- file.path("temp.yaml") + # Define the data + data <- list( + correct_true = "true", + correct_false = "false", + false_true = "Y", + false_false = "N", + false_true2 = "yes", + false_false2 = "no" + ) + write_yaml(data, temp_file) + + ### Removal of quotes necessary + # Read the file + file_content <- readLines(temp_file) + + # Apply noquote to each line + file_content <- gsub("'", "", file_content) + + # Write the modified content back to a file + writeLines(as.character(file_content), temp_file) + + ### Load temp params.yaml + # Load in temporary YAML file using read_safe_yaml + safe_yaml <- read_safe_yaml(temp_file) + + # Load in temporary YAML file using default read_yaml + yaml <- read_yaml(temp_file) + + # Test default read_yaml function + expect_false(yaml$false_true == data$false_true) + expect_false(yaml$false_false == data$false_false) + expect_false(yaml$false_true2 == data$false_true2) + expect_false(yaml$false_false2 == data$false_false2) + expect_true(yaml$correct_true == TRUE) + expect_true(yaml$correct_false == FALSE) + + # Test read_safe_yaml function + expect_true(safe_yaml$false_true == safe_yaml$false_true) + expect_true(safe_yaml$false_false == safe_yaml$false_false) + expect_true(safe_yaml$false_true2 == safe_yaml$false_true2) + expect_true(safe_yaml$false_false2 == safe_yaml$false_false2) + expect_true(safe_yaml$correct_true == TRUE) + expect_true(safe_yaml$correct_false == FALSE) + + file.remove(temp_file) +}) From 6c7753234f0b95507bb27d8bc0b61327d173ff08 Mon Sep 17 00:00:00 2001 From: "zxBIB Schreyer,Daniel (TMCP TM&BM P) EXTERNAL" Date: Mon, 13 Jan 2025 13:42:52 +0100 Subject: [PATCH 5/7] fixed issues --- .Rbuildignore | 2 ++ R/read_params.R | 3 +-- tests/testthat/test-read_params.R | 12 ++++++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.Rbuildignore b/.Rbuildignore index 0b5bd85..c405465 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -1,3 +1,5 @@ +^renv$ +^renv\.lock$ ^.*\.Rproj$ ^\.Rproj\.user$ ^\.pre-commit-config\.yaml$ diff --git a/R/read_params.R b/R/read_params.R index d71484d..e770ea7 100644 --- a/R/read_params.R +++ b/R/read_params.R @@ -72,7 +72,7 @@ read_params <- function(stage_path = NULL, return_list = FALSE) { } ) - yaml <- read_clean_yaml(tmp_config_file) + yaml <- read_safe_yaml(tmp_config_file) unlink(tmp_config_file) if (return_list) { @@ -132,7 +132,6 @@ stage_here <- function(...) { #' @return a list #' @importFrom yaml read_yaml #' @importFrom purrr modify_tree -#' @export read_safe_yaml <- function(params_file) { yaml <- read_yaml(params_file, handlers = list( diff --git a/tests/testthat/test-read_params.R b/tests/testthat/test-read_params.R index 17ff5a9..843eb98 100644 --- a/tests/testthat/test-read_params.R +++ b/tests/testthat/test-read_params.R @@ -31,6 +31,8 @@ test_that("read_params: yes and no parameters are correctly loaded", { # Load in temporary YAML file using default read_yaml yaml <- read_yaml(temp_file) + file.remove(temp_file) + # Test default read_yaml function expect_false(yaml$false_true == data$false_true) expect_false(yaml$false_false == data$false_false) @@ -40,12 +42,10 @@ test_that("read_params: yes and no parameters are correctly loaded", { expect_true(yaml$correct_false == FALSE) # Test read_safe_yaml function - expect_true(safe_yaml$false_true == safe_yaml$false_true) - expect_true(safe_yaml$false_false == safe_yaml$false_false) - expect_true(safe_yaml$false_true2 == safe_yaml$false_true2) - expect_true(safe_yaml$false_false2 == safe_yaml$false_false2) + expect_true(safe_yaml$false_true == data$false_true) + expect_true(safe_yaml$false_false == data$false_false) + expect_true(safe_yaml$false_true2 == data$false_true2) + expect_true(safe_yaml$false_false2 == data$false_false2) expect_true(safe_yaml$correct_true == TRUE) expect_true(safe_yaml$correct_false == FALSE) - - file.remove(temp_file) }) From 2dfb1b6d2b9de8893765be9cdc8a92a861221dfa Mon Sep 17 00:00:00 2001 From: "zxBIB Schreyer,Daniel (TMCP TM&BM P) EXTERNAL" Date: Mon, 13 Jan 2025 13:54:12 +0100 Subject: [PATCH 6/7] added read_safe_yaml doc and NAMESPACE --- NAMESPACE | 1 - man/read_safe_yaml.Rd | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 man/read_safe_yaml.Rd diff --git a/NAMESPACE b/NAMESPACE index 2fdd9d9..07261a9 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -10,7 +10,6 @@ export(dso_repro_stage_addin) export(dso_repro_stage_w_dependencies_addin) export(init) export(read_params) -export(read_safe_yaml) export(reload) export(repro) export(safe_get) diff --git a/man/read_safe_yaml.Rd b/man/read_safe_yaml.Rd new file mode 100644 index 0000000..555dd30 --- /dev/null +++ b/man/read_safe_yaml.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/read_params.R +\name{read_safe_yaml} +\alias{read_safe_yaml} +\title{read_safe_yaml} +\usage{ +read_safe_yaml(params_file) +} +\arguments{ +\item{params_file}{path to params.yaml file} +} +\value{ +a list +} +\description{ +Safely reads in yaml files +} From ea683e345d1f44ef73a3c04b5acc85290e7703c1 Mon Sep 17 00:00:00 2001 From: grst <7051479+grst@users.noreply.github.com> Date: Mon, 20 Jan 2025 11:02:04 +0000 Subject: [PATCH 7/7] pre-commit autofixes --- NAMESPACE | 2 +- man/read_safe_yaml.Rd | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index eea6392..b37014a 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -10,6 +10,7 @@ export(dso_repro_stage_addin) export(dso_repro_stage_w_dependencies_addin) export(init) export(read_params) +export(read_safe_yaml) export(reload) export(repro) export(set_stage) @@ -22,6 +23,5 @@ importFrom(here,here) importFrom(here,i_am) importFrom(methods,show) importFrom(purrr,modify_tree) -importFrom(rlang,caller_env) importFrom(rstudioapi,viewer) importFrom(yaml,read_yaml) diff --git a/man/read_safe_yaml.Rd b/man/read_safe_yaml.Rd index 555dd30..96bbffb 100644 --- a/man/read_safe_yaml.Rd +++ b/man/read_safe_yaml.Rd @@ -6,9 +6,6 @@ \usage{ read_safe_yaml(params_file) } -\arguments{ -\item{params_file}{path to params.yaml file} -} \value{ a list }