Skip to content

Commit

Permalink
warn on unused arguments in standardize_case_names() (#690)
Browse files Browse the repository at this point in the history
  • Loading branch information
JanMarvin authored Jul 15, 2023
1 parent f9163e4 commit 46a1080
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 21 deletions.
6 changes: 4 additions & 2 deletions R/class-workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -4069,7 +4069,8 @@ wbWorkbook <- R6::R6Class(
...
) {

standardize_case_names(...)
arguments <- c(ls(), "start_row", "start_col")
standardize_case_names(..., arguments = arguments)

if (exists("start_row") && !is.null(start_row) &&
exists("start_col") && !is.null(start_col)) {
Expand Down Expand Up @@ -5391,7 +5392,8 @@ wbWorkbook <- R6::R6Class(
...
) {

standardize_case_names(...)
arguments <- c(ls(), "rows", "cols")
standardize_case_names(..., arguments = arguments)

sheet <- private$get_sheet_index(sheet)

Expand Down
15 changes: 13 additions & 2 deletions R/standardize.R
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ standardize_color_names <- function(..., return = FALSE) {
#' @param ... ...
#' @returns void. assigns an object in the parent frame
#' @noRd
standardize_case_names <- function(..., return = FALSE) {
standardize_case_names <- function(..., return = FALSE, arguments = NULL) {

if (is.null(arguments)) {
arguments <- ls(envir = parent.frame())
}

# since R 4.1.0: ...names()
args <- list(...)
Expand All @@ -58,6 +62,7 @@ standardize_case_names <- function(..., return = FALSE) {
x = camel_case,
perl = TRUE
)
got[got_camel_case] <- name_camel_case
# since R 3.5.0: ...elt(got_col)
if (return) {
names(args)[got_camel_case] <- name_camel_case
Expand All @@ -68,6 +73,11 @@ standardize_case_names <- function(..., return = FALSE) {
}
}

sel <- !got %in% arguments
if (any(sel)) {
warning("unused arguments (", paste(got[sel], collapse = ", "), ")")
}

if (return) args

}
Expand All @@ -79,9 +89,10 @@ standardize_case_names <- function(..., return = FALSE) {
standardize <- function(...) {

nms <- list(...)
arguments <- ls(envir = parent.frame())

rtns <- standardize_color_names(nms, return = TRUE)
rtns <- standardize_case_names(rtns, return = TRUE)
rtns <- standardize_case_names(rtns, return = TRUE, arguments = arguments)

nms <- names(rtns)
for (i in seq_along(nms)) {
Expand Down
2 changes: 1 addition & 1 deletion R/wb_load.R
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ wb_load <- function(
overrideAttr <- as.data.frame(do.call("rbind", override))
xmls <- basename(unlist(overrideAttr$PartName))
drawings <- grep("drawing", xmls, value = TRUE)
wb$add_worksheet(sheets$name[i], visible = is_visible[i], hasDrawing = !is.na(drawings[i]))
wb$add_worksheet(sheets$name[i], visible = is_visible[i], has_drawing = !is.na(drawings[i]))
}
}

Expand Down
3 changes: 2 additions & 1 deletion R/wb_styles.R
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,9 @@ create_cell_style <- function(
) {
n <- length(num_fmt_id)

arguments <- c(ls(), "is_cell_style_xf")
standardize_case_names(..., arguments = arguments)
args <- list(...)
standardize_case_names(...)

is_cell_style_xf <- isTRUE(args$is_cell_style_xf)

Expand Down
14 changes: 12 additions & 2 deletions R/write_xlsx.R
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,23 @@ write_xlsx <- function(x, file, as_table = FALSE, ...) {

## set scientific notation penalty

arguments <- c(ls(), "creator", "sheet_name", "grid_lines",
"tab_color", "tab_colour",
"zoom", "header", "footer", "even_header", "even_footer", "first_header",
"first_footer", "start_col", "start_row",
"col.names", "row.names", "col_names", "row_names", "table_style",
"table_name", "with_filter", "first_active_row", "first_active_col",
"first_row", "first_col", "col_widths", "na.strings",
"overwrite", "title", "subject", "category"
)

params <- list(...)

# we need them in params
params <- standardize_case_names(params, return = TRUE)
params <- standardize_case_names(params, arguments = arguments, return = TRUE)

# and in global env for `asTable`
standardize_case_names(...)
standardize_case_names(..., arguments = arguments)

## Possible parameters

Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-class-hyperlink.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ test_that("encode Hyperlink works", {
add_worksheet("Tab_1", zoom = 80, gridLines = FALSE)$
add_data(x = rbind(2016:2019), dims = "C1:F1", colNames = FALSE)$
add_data(x = 2017, dims = "A1", colNames = FALSE)$
add_data_validation(rows = 1, col = 1, type = "list", value = '"2016,2017,2018,2019"')$
add_data_validation(dims = "A1", type = "list", value = '"2016,2017,2018,2019"')$
add_formula(dims = "B1", x = formula_old)$
add_formula(dims = "B2", x = formula_new)

Expand All @@ -32,7 +32,7 @@ test_that("formulas with hyperlinks works", {
add_worksheet("Tab_1", zoom = 80, gridLines = FALSE)$
add_data(dims = "C1:F1", x = rbind(2016:2019), colNames = FALSE)$
add_data(x = 2017, startCol = 1, startRow = 1, colNames = FALSE)$
add_data_validation(rows = 1, col = 1, type = "list", value = '"2016,2017,2018,2019"')$
add_data_validation(dims = "A1", type = "list", value = '"2016,2017,2018,2019"')$
add_formula(dims = "B1", x = '=HYPERLINK("#Tab_1!" & CELL("address", INDEX(C1:F1, MATCH(A1, C1:F1, 0))), "Go to the selected column")')$
add_formula(dims = "B2", x = '=IF(2017 = VALUE(A1), HYPERLINK("github.com","github.com"), A1)')

Expand Down
10 changes: 5 additions & 5 deletions tests/testthat/test-class-workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ test_that("data validation", {
wb <- wb_workbook()$
add_worksheet("Sheet 1")$
add_data_table(x = head(iris))$
add_data_validation(col = "A", rows = 2:151, type = "whole",
add_data_validation(dims = "A2:A151", type = "whole",
operator = "between", value = c(1, 9, 19)
),
"length <= 2"
Expand All @@ -246,7 +246,7 @@ test_that("data validation", {
wb <- wb_workbook()$
add_worksheet("Sheet 1")$
add_data_table(x = head(iris))$
add_data_validation(col = "A", rows = 2:151, type = "even",
add_data_validation(dims = "A2:A151", type = "even",
operator = "between", value = c(1, 9)
),
"Invalid 'type' argument!"
Expand All @@ -257,7 +257,7 @@ test_that("data validation", {
wb <- wb_workbook()$
add_worksheet("Sheet 1")$
add_data_table(x = head(iris))$
add_data_validation(col = "A", rows = 2:151, type = "whole",
add_data_validation(dims = "A2:A151", type = "whole",
operator = "lower", value = c(1, 9)
),
"Invalid 'operator' argument!"
Expand All @@ -269,7 +269,7 @@ test_that("data validation", {
add_worksheet("Sheet 1")$
add_data_table(x = head(iris))$
# whole numbers are fine
add_data_validation(col = 1, rows = 2:12, type = "date",
add_data_validation(dims = "A2:A12", type = "date",
operator = "greaterThanOrEqual", value = 7
),
"If type == 'date' value argument must be a Date vector"
Expand All @@ -281,7 +281,7 @@ test_that("data validation", {
add_worksheet("Sheet 1")$
add_data_table(x = head(iris))$
# whole numbers are fine
add_data_validation(col = 1, rows = 2:12, type = "time",
add_data_validation(dims = "A2:A12", type = "time",
operator = "greaterThanOrEqual", value = 7
),
"If type == 'time' value argument must be a POSIXct or POSIXlt vector."
Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-class-worksheet.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ test_that("test data validation list and sparklines", {
wb <- wb_workbook()$
add_worksheet()$add_data(x = iris[1:30, ])$
add_worksheet()$add_data(sheet = 2, x = sample(iris$Sepal.Length, 10))$
add_data_validation(sheet = 1, col = 1, rows = 2:11, type = "list", value = '"O1,O2"')$
add_data_validation(sheet = 1, dims = "A2:A11", type = "list", value = '"O1,O2"')$
add_sparklines(sheet = 1, sparklines = s1)$
add_data_validation(sheet = 1, col = 1, rows = 12:21, type = "list", value = '"O2,O3"')$
add_data_validation(sheet = 1, dims = "A12:A21", type = "list", value = '"O2,O3"')$
add_sparklines(sheet = 1, sparklines = s2)

exp <- c(
Expand All @@ -35,7 +35,7 @@ test_that("old and new data validations", {
add_worksheet()$
add_data(x = sample(c("O1", "O2"), 10, TRUE))$
add_data(dims = "B1", x = sample(c("O1", "O2"), 10, TRUE))$
add_data_validation(sheet = 1, col = 2, rows = 1:10, type = "list", value = '"O1,O2"')
add_data_validation(sheet = 1, dims = "B1:B10", type = "list", value = '"O1,O2"')

# add data validations list as x14. this was the default in openxlsx and openxlsx2 <= 0.3
wb$worksheets[[1]]$extLst <- "<ext xmlns:x14=\"http://schemas.microsoft.com/office/spreadsheetml/2009/9/main\" uri=\"{CCE6A557-97BC-4b89-ADB6-D9C93CAAB3DF}\"><x14:dataValidations xmlns:xm=\"http://schemas.microsoft.com/office/excel/2006/main\" count=\"2\"><x14:dataValidation type=\"list\" allowBlank=\"1\" showInputMessage=\"1\" showErrorMessage=\"1\"><x14:formula1><xm:f>\"O1,O2\"</xm:f></x14:formula1><xm:sqref>A2:A11</xm:sqref></x14:dataValidation></x14:dataValidations></ext>"
Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-save.R
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ test_that("regression test for #248", {
tempFile <- temp_xlsx()

# no formatting
expect_silent(write_xlsx(df, tempFile, borders = "columns", overwrite = TRUE))
expect_silent(write_xlsx(df, tempFile, overwrite = TRUE))

# Change column class to percentage
class(df$percent) <- "percentage"
expect_silent(write_xlsx(df, tempFile, borders = "columns", overwrite = TRUE))
expect_silent(write_xlsx(df, tempFile, overwrite = TRUE))
})


Expand Down Expand Up @@ -133,7 +133,7 @@ test_that("write xlsx", {
expect_silent(write_xlsx(df, tmp, row.names = TRUE))
expect_error(write_xlsx(df, tmp, rowNames = "NO"))
expect_silent(write_xlsx(df, tmp, rowNames = TRUE))
expect_silent(write_xlsx(df, tmp, colWidth = "auto"))
expect_silent(write_xlsx(df, tmp, colWidths = "auto"))
expect_silent(write_xlsx(list(df, df), tmp, firstActiveCol = 2, firstActiveRow = 2))
expect_silent(write_xlsx(list(df, df), tmp, firstCol = FALSE, firstRow = FALSE))
expect_silent(write_xlsx(list(df, df), tmp, firstCol = TRUE, firstRow = TRUE))
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-standardize.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ test_that("standardize works", {
expect_equal(get("tabColor"), "green")

camelCase <- NULL
camel_case <- NULL
standardize_case_names(camelCase = "green")
expect_equal(get("camel_case"), "green")

Expand Down

0 comments on commit 46a1080

Please sign in to comment.