Skip to content

Commit

Permalink
163 since format is printed, we may want to improve how formats are p…
Browse files Browse the repository at this point in the history
…rinted, currently it is (#164)

#163

close #163 

Improve the print method to display `<NA>` when the value is `NA` and
surround strings with `"` to highlight the fact that they are character
and allow to better visualize with spaces.

thank you for the review
  • Loading branch information
BFalquet authored Oct 22, 2024
1 parent 77c7368 commit 31df4f8
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
3 changes: 2 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

* Rules specified under the `all_datasets` keyword in a format list will apply to every data set of the reformatted object unless specified otherwise.
* New `verbose` argument in the `reformat` method. When applied to `list` the value of this augment can be controlled with the `dunlin.reformat.verbose` option or the `R_DUNLIN_REFORMAT_VERBOSE` environment variable.

* Improve the output when printing `rule` objects.

# dunlin 0.1.6

* `render_safe` now renders placeholder using in priority values corresponding to the key matching exactly the placeholder, case included.
Expand Down
13 changes: 9 additions & 4 deletions R/rules.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,21 @@ rule <- function(..., .lst = list(...), .string_as_fct = TRUE, .na_last = TRUE,
#'
print.rule <- function(x, ...) {
cat("Mapping of:\n")
nms <- names(x)
nms <- unique(names(x))
if (length(x) == 0) {
cat("Empty mapping.\n")
} else {
for (i in seq_len(length(x))) {
cat(nms[i], " <- ", if (length(x[[i]]) > 1) sprintf("[%s]", toString(x[[i]])) else x[[i]], "\n")
for (i in nms) {
ori_nms <- unlist(x[names(x) %in% i])
ori_nms <- ifelse(is.na(ori_nms), "<NA>", stringr::str_c("\"", ori_nms, "\""))
ori_nms <- toString(ori_nms)
cat(i, " <- ", ori_nms, "\n")
}
}
.to_NA <- attr(x, ".to_NA")
if (!is.null(.to_NA)) cat("NA <- ", toString(.to_NA), "\n")
if (!is.null(.to_NA)) {
cat("Convert to <NA>:", toString(stringr::str_c("\"", .to_NA, "\"")), "\n")
}
cat("Convert to factor:", attr(x, ".string_as_fct"), "\n")
cat("Drop unused level:", attr(x, ".drop"), "\n")
cat("NA-replacing level in last position:", attr(x, ".na_last"), "\n")
Expand Down
21 changes: 18 additions & 3 deletions tests/testthat/_snaps/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,24 @@
rule(a = "1", b = NA)
Output
Mapping of:
a <- 1
b <- NA
NA <-
a <- "1"
b <- <NA>
Convert to <NA>: ""
Convert to factor: TRUE
Drop unused level: FALSE
NA-replacing level in last position: TRUE

# rule with multiple matching printed correctly

Code
rule(a = "1", b = c(NA, "b"), x = c("first", "second"), .to_NA = c("",
"missing"))
Output
Mapping of:
a <- "1"
b <- <NA>, "b"
x <- "first", "second"
Convert to <NA>: "", "missing"
Convert to factor: TRUE
Drop unused level: FALSE
NA-replacing level in last position: TRUE
Expand Down
6 changes: 6 additions & 0 deletions tests/testthat/test-rules.R
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,16 @@ test_that("rule fails for values that is not character/logical/numeric", {
)
})

# print ----

test_that("rule printed correctly", {
expect_snapshot(rule(a = "1", b = NA))
})

test_that("rule with multiple matching printed correctly", {
expect_snapshot(rule(a = "1", b = c(NA, "b"), x = c("first", "second"), .to_NA = c("", "missing")))
})

# list2rules ----

test_that("list2rules works as expected", {
Expand Down

0 comments on commit 31df4f8

Please sign in to comment.