From 72a72b85cfa07ffdd7c2ca8115da5e8daeaaea10 Mon Sep 17 00:00:00 2001 From: infra-bot Date: Fri, 3 Jul 2026 01:27:43 +0200 Subject: [PATCH] fix: equal_mode normalized implique case_insensitive + trim sauf override (issue #27) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le champ n'etait lu qu'a un endroit ou identical(eq_mode, "normalized") activait normalize_text(case_insensitive = FALSE, trim = FALSE) - l'identite. Un utilisateur ecrivant equal_mode: normalized seul n'obtenait rien, silencieusement, et test-utils verrouillait meme l'inertie. La vignette documentait pourtant deja « Apply both transformations » : le code s'aligne sur sa doc. - normalized implique les deux normalisations pour les colonnes character, un flag explicite (meme FALSE) gagne sur le mode - date/datetime/logical : *_equal_mode documentes acceptes-mais-inertes (la normalisation ne touche que character) - test verrouillant l'inertie retourne + reprex de l'issue en test --- NEWS.md | 14 ++++++++ R/compare_datasets_from_yaml.R | 24 +++++++++++-- R/preprocessing.R | 20 ++++++++--- man/write_rules_template.Rd | 15 +++++--- tests/testthat/test-utils.R | 64 +++++++++++++++++++++++++++++++--- 5 files changed, 121 insertions(+), 16 deletions(-) diff --git a/NEWS.md b/NEWS.md index 73be078..a306e59 100644 --- a/NEWS.md +++ b/NEWS.md @@ -88,6 +88,20 @@ ## Bug fixes +* `equal_mode: normalized` now does what the vignette always said: it implies + `case_insensitive = TRUE` and `trim = TRUE` for character columns unless + those flags are set explicitly (an explicit value wins over the mode). It + previously activated a branch that applied the identity transformation: a + user writing `equal_mode: normalized` alone got nothing, silently, and a + test even locked that inertia. `write_rules_template()` stops co-writing + the default FALSE flags next to a "normalized" mode (they neutralised the + implication). Note: a hand-written YAML using `equal_mode: normalized` + without explicit flags now normalizes where it previously compared exact, + so verdicts on such configs can flip from FAIL to PASS; re-run rather than + assume stability. The `date`/`datetime`/`logical` equal-mode parameters are + documented as accepted-but-inert for non-character types (text + normalization only touches character columns) (issue #27). + * Factor columns are now compared as the character values they display: preprocessing converts them, so the text normalization rules (`case_insensitive`, `trim`) apply to them and the verdict no longer depends diff --git a/R/compare_datasets_from_yaml.R b/R/compare_datasets_from_yaml.R index 91f3565..259d554 100644 --- a/R/compare_datasets_from_yaml.R +++ b/R/compare_datasets_from_yaml.R @@ -18,12 +18,19 @@ #' @param numeric_abs Default absolute tolerance for numeric columns #' @param numeric_rel Default relative tolerance for numeric columns #' @param integer_abs Default absolute tolerance for integer columns -#' @param character_equal_mode Default comparison mode for character columns ("exact", "normalized") +#' @param character_equal_mode Default comparison mode for character columns: +#' "exact", or "normalized" which implies `case_insensitive = TRUE` and +#' `trim = TRUE` unless those flags are set explicitly (an explicit value +#' always wins over the mode) #' @param character_case_insensitive Logical for case-insensitive character comparison #' @param character_trim Logical for trimming whitespace in character comparison -#' @param date_equal_mode Default comparison mode for date columns +#' @param date_equal_mode Default comparison mode for date columns. Only +#' "exact" has an effect: text normalization applies to character columns +#' only, so "normalized" is accepted but changes nothing for this type. #' @param datetime_equal_mode Default comparison mode for datetime columns +#' (same caveat as `date_equal_mode`) #' @param logical_equal_mode Default comparison mode for logical columns +#' (same caveat as `date_equal_mode`) #' @return The \code{path} to the written YAML file, returned invisibly. #' @importFrom yaml write_yaml #' @importFrom stats setNames @@ -67,6 +74,17 @@ write_rules_template <- function(data_reference, } types <- detect_column_types(.ref_schema) + # With equal_mode "normalized", writing the default FALSE flags would + # neutralise the mode's implication (an explicit flag wins over the mode): + # only include the flags the caller actually supplied + character_rules <- list(equal_mode = character_equal_mode) + if (!identical(character_equal_mode, "normalized") || + !missing(character_case_insensitive)) { + character_rules$case_insensitive <- character_case_insensitive + } + if (!identical(character_equal_mode, "normalized") || !missing(character_trim)) { + character_rules$trim <- character_trim + } y <- list( version = version, defaults = list(na_equal = na_equal_default, @@ -78,7 +96,7 @@ write_rules_template <- function(data_reference, by_type = list( numeric = list(abs = numeric_abs, rel = numeric_rel), integer = list(abs = integer_abs), - character = list(equal_mode = character_equal_mode, case_insensitive = character_case_insensitive, trim = character_trim), + character = character_rules, date = list(equal_mode = date_equal_mode), datetime = list(equal_mode = datetime_equal_mode), logical = list(equal_mode = logical_equal_mode) diff --git a/R/preprocessing.R b/R/preprocessing.R index 678068d..10e7c54 100644 --- a/R/preprocessing.R +++ b/R/preprocessing.R @@ -65,12 +65,22 @@ preprocess_dataframe <- function(df, col_rules, schema = NULL) { } for (nm in names(col_rules)) { cr <- col_rules[[nm]] - eq_mode <- cr$equal_mode %||% "exact" - case_insensitive <- isTRUE(cr$case_insensitive) - trim <- isTRUE(cr$trim) + normalized <- identical(cr$equal_mode %||% "exact", "normalized") - # Apply normalization if equal_mode is "normalized" OR if case_insensitive/trim is TRUE - should_normalize <- identical(eq_mode, "normalized") || case_insensitive || trim + # equal_mode "normalized" implies BOTH text normalizations unless the + # rule sets them explicitly (an explicit FALSE wins over the mode) + case_insensitive <- if (is.null(cr$case_insensitive)) { + normalized + } else { + isTRUE(cr$case_insensitive) + } + trim <- if (is.null(cr$trim)) { + normalized + } else { + isTRUE(cr$trim) + } + + should_normalize <- case_insensitive || trim # Determine column type: use schema for lazy tables, otherwise inspect df # directly. A factor in the schema counts as character: the local values diff --git a/man/write_rules_template.Rd b/man/write_rules_template.Rd index 30e42c9..b153de3 100644 --- a/man/write_rules_template.Rd +++ b/man/write_rules_template.Rd @@ -54,17 +54,24 @@ comparison. If NULL, comparison is positional (row by row).} \item{integer_abs}{Default absolute tolerance for integer columns} -\item{character_equal_mode}{Default comparison mode for character columns ("exact", "normalized")} +\item{character_equal_mode}{Default comparison mode for character columns: +"exact", or "normalized" which implies \code{case_insensitive = TRUE} and +\code{trim = TRUE} unless those flags are set explicitly (an explicit value +always wins over the mode)} \item{character_case_insensitive}{Logical for case-insensitive character comparison} \item{character_trim}{Logical for trimming whitespace in character comparison} -\item{date_equal_mode}{Default comparison mode for date columns} +\item{date_equal_mode}{Default comparison mode for date columns. Only +"exact" has an effect: text normalization applies to character columns +only, so "normalized" is accepted but changes nothing for this type.} -\item{datetime_equal_mode}{Default comparison mode for datetime columns} +\item{datetime_equal_mode}{Default comparison mode for datetime columns +(same caveat as \code{date_equal_mode})} -\item{logical_equal_mode}{Default comparison mode for logical columns} +\item{logical_equal_mode}{Default comparison mode for logical columns +(same caveat as \code{date_equal_mode})} } \value{ The \code{path} to the written YAML file, returned invisibly. diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index b56b33a..128dec1 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -47,15 +47,42 @@ test_that("preprocess_dataframe applies transformations correctly", { expect_equal(result$case_col, c("mixed", "case")) }) -test_that("preprocess_dataframe handles equal_mode normalized", { +test_that("equal_mode normalized implies case_insensitive and trim by default", { df <- data.frame(text_col = c(" HELLO ", "WORLD")) rules <- list(text_col = list(equal_mode = "normalized")) - result <- preprocess_dataframe(df, rules) + expect_equal(result$text_col, c("hello", "world")) + + # An explicit flag overrides the implied default + rules_no_trim <- list(text_col = list(equal_mode = "normalized", trim = FALSE)) + result_no_trim <- preprocess_dataframe(df, rules_no_trim) + expect_equal(result_no_trim$text_col, c(" hello ", "world")) - # Should not apply any transformation since case_insensitive and trim are not set - expect_equal(result$text_col, c(" HELLO ", "WORLD")) + rules_no_case <- list(text_col = list(equal_mode = "normalized", + case_insensitive = FALSE)) + result_no_case <- preprocess_dataframe(df, rules_no_case) + expect_equal(result_no_case$text_col, c("HELLO", "WORLD")) +}) + +test_that("equal_mode normalized alone changes the verdict (issue reprex)", { + ref <- data.frame(id = 1:2, s = c("A", " b"), stringsAsFactors = FALSE) + cand <- data.frame(id = 1:2, s = c("a", "b"), stringsAsFactors = FALSE) + + yaml_path <- tempfile(fileext = ".yaml") + on.exit(unlink(yaml_path), add = TRUE) + writeLines(' +version: 1 +defaults: {keys: [id], na_equal: true} +row_validation: {check_count: false} +by_type: + character: {equal_mode: normalized} +', con = yaml_path) + + res <- suppressMessages( + compare_datasets_from_yaml(ref, cand, key = "id", path = yaml_path) + ) + expect_true(res$all_passed) }) test_that("preprocess_dataframe handles mixed rules", { @@ -112,3 +139,32 @@ test_that("preprocess_dataframe applies stringr trim on Arrow objects", { expect_equal(result$name, c("Alice", "Bob")) }) + +test_that("a normalized template does not neutralise its own mode", { + ref <- data.frame(id = 1:2, s = c("A", " b"), stringsAsFactors = FALSE) + cand <- data.frame(id = 1:2, s = c("a", "b"), stringsAsFactors = FALSE) + + yaml_path <- tempfile(fileext = ".yaml") + on.exit(unlink(yaml_path), add = TRUE) + write_rules_template(ref, key = "id", path = yaml_path, + character_equal_mode = "normalized") + + rules <- read_rules(yaml_path) + # The default FALSE flags must not be co-written next to the mode + expect_null(rules$by_type$character$case_insensitive) + expect_null(rules$by_type$character$trim) + + res <- suppressMessages( + compare_datasets_from_yaml(ref, cand, key = "id", path = yaml_path) + ) + expect_true(res$all_passed) + + # An explicitly supplied flag still wins over the mode + yaml_path2 <- tempfile(fileext = ".yaml") + on.exit(unlink(yaml_path2), add = TRUE) + write_rules_template(ref, key = "id", path = yaml_path2, + character_equal_mode = "normalized", + character_trim = FALSE) + rules2 <- read_rules(yaml_path2) + expect_false(rules2$by_type$character$trim) +})