[misc] housekeeping deprecating (#1330)

* [misc] deprecate create_comment()

* [misc] deprecate argument usage of rows/cols

* [misc] prepare deprecation of camelCase conversion

Author: JanMarvin

NEWS.md

@@ -1,5 +1,10 @@
 # openxlsx2 (development version)
 
+## Maintenance
+
+* Legacy `create_comment()` is now deprecated in favor of `wb_comment()`.
+* Usage of `rows` and `cols` is now deprecated in favor of `wb_dims(rows = ..., cols = ...)`.
+
 ## New features
 
 * A normalization function was added for separators in formulas. In English locales, formulas require a comma as argument separator, locales that use the comma a decimal separator, a semicolon is used as argument separator. We added a check to replace semicolons added in formulas, because these were hard to spot and if overlooked could brick the XML worksheet.

R/class-comment.R

@@ -164,29 +164,6 @@ wb_comment <- function(
   invisible(wbComment$new(text = text, author = author, style = style, width = width, height = height, visible = visible))
 }
 
-#' Create a comment
-#'
-#' Use [wb_comment()] in new code. See [openxlsx2-deprecated]
-#'
-#' @inheritParams wb_comment
-#' @param author A string, by default, will use "user"
-#' @param visible Default: `TRUE`. Is the comment visible by default?
-#' @keywords internal
-#' @returns a `wbComment` object
-#' @export
-create_comment <- function(text,
-  author = Sys.info()[["user"]],
-  style = NULL,
-  visible = TRUE,
-  width = 2,
-  height = 4) {
-  #
-  if (getOption("openxlsx2.soon_deprecated", default = FALSE)) {
-    .Deprecated("wb_comment()", old = "create_comment()")
-  }
-  wb_comment(text = text, author = author, style = style, visible = visible, width = width[1], height = height[1])
-}
-
 do_write_comment <- function(
     wb,
     sheet,

R/class-workbook.R

@@ -5321,13 +5321,7 @@ wbWorkbook <- R6::R6Class(
       rows <- list(...)[["rows"]]
 
       if (!is.null(rows) && !is.null(cols)) {
-
-        if (length(cols) > 2 && any(diff(cols) != 1))
-          warning("cols > 2, will create range from min to max.")
-
-        if (getOption("openxlsx2.soon_deprecated", default = FALSE))
-          .Deprecated(old = "cols/rows", new = "dims", package = "openxlsx2")
-
+        .Deprecated(old = "cols/rows", new = "dims", package = "openxlsx2")
         dims <- rowcol_to_dims(rows, cols)
       }
 
@@ -5345,13 +5339,7 @@ wbWorkbook <- R6::R6Class(
       rows <- list(...)[["rows"]]
 
       if (!is.null(rows) && !is.null(cols)) {
-
-        if (length(cols) > 2 && any(diff(cols) != 1))
-          warning("cols > 2, will create range from min to max.")
-
-        if (getOption("openxlsx2.soon_deprecated", default = FALSE))
-          .Deprecated(old = "cols/rows", new = "dims", package = "openxlsx2")
-
+        .Deprecated(old = "cols/rows", new = "dims", package = "openxlsx2")
         dims <- rowcol_to_dims(rows, cols)
       }
 
@@ -5796,20 +5784,17 @@ wbWorkbook <- R6::R6Class(
       rows <- list(...)[["rows"]]
 
       if (!is.null(rows) && !is.null(cols)) {
-
-        if (length(cols) > 2 && any(diff(cols) != 1))
-          warning("cols > 2, will create range from min to max.")
-
-        if (getOption("openxlsx2.soon_deprecated", default = FALSE))
-          .Deprecated(old = "cols/rows", new = "dims", package = "openxlsx2")
-
+        .Deprecated(old = "cols/rows", new = "dims", package = "openxlsx2")
         dims <- rowcol_to_dims(rows, cols)
       }
 
       ddims <- dims_to_rowcol(dims, as_integer = TRUE)
       rows <- ddims[["row"]]
       cols <- ddims[["col"]]
 
+      if (length(cols) > 2 && any(diff(cols) != 1))
+        warning("cols > 2, will create range from min to max.")
+
       if (!is.null(style)) assert_class(style, "character")
       assert_class(type, "character")
       assert_class(params, "list")

R/class-worksheet.R

@@ -523,10 +523,13 @@ wbWorksheet <- R6::R6Class(
       ddims <- dims_to_rowcol(dims)
 
       rows <- ddims[["row"]]
-      cols <- ddims[["col"]]
+      cols <- col2int(ddims[["col"]])
+
+      if (length(cols) > 2 && any(diff(cols) != 1))
+        warning("cols > 2, will create range from min to max.")
 
       rows <- range(as.integer(rows))
-      cols <- range(col2int(cols))
+      cols <- range(cols)
 
       sqref <- paste0(int2col(cols), rows)
       sqref <- stringi::stri_join(sqref, collapse = ":", sep = " ")
@@ -582,8 +585,13 @@ wbWorksheet <- R6::R6Class(
     #' @return The `wbWorkbook` object, invisibly
     unmerge_cells = function(rows = NULL, cols = NULL) {
 
+      cols <- col2int(cols)
+
+      if (length(cols) > 2 && any(diff(cols) != 1))
+        warning("cols > 2, will create range from min to max.")
+
       rows <- range(as.integer(rows))
-      cols <- range(col2int(cols))
+      cols <- range(cols)
 
       sqref <- paste0(int2col(cols), rows)
       sqref <- stringi::stri_join(sqref, collapse = ":", sep = " ")

R/deprecate.R

@@ -6,6 +6,26 @@
 #' @name comment_internal
 NULL
 
+#' Create a comment
+#'
+#' Use [wb_comment()] in new code. See [openxlsx2-deprecated]
+#'
+#' @inheritParams wb_comment
+#' @param author A string, by default, will use "user"
+#' @param visible Default: `TRUE`. Is the comment visible by default?
+#' @keywords internal
+#' @returns a `wbComment` object
+#' @export
+create_comment <- function(text,
+                           author = Sys.info()[["user"]],
+                           style = NULL,
+                           visible = TRUE,
+                           width = 2,
+                           height = 4) {
+  .Deprecated("wb_comment()", old = "create_comment()")
+  wb_comment(text = text, author = author, style = style, visible = visible, width = width[1], height = height[1])
+}
+
 #' @rdname comment_internal
 #' @inheritParams wb_add_comment
 #' @param comment An object created by [create_comment()]

man/create_comment.Rd

@@ -12,7 +12,7 @@ test_that("get_base_font works", {
     )
   )
 
-  wb$set_base_font(fontSize = 9, fontName = "Arial", fontColour = wb_colour("red"))
+  wb$set_base_font(font_size = 9, font_name = "Arial", font_colour = wb_colour("red"))
   expect_equal(
     wb$get_base_font(),
     list(

tests/testthat/test-base_font.R

@@ -21,18 +21,18 @@ test_that("tabColour can be wb_colour()", {
   expect_silent(
     wb_workbook()$
       # wb_colour
-      add_worksheet(tabColour = wb_colour("green"))$
-      add_chartsheet(tabColour = wb_colour("green"))$
+      add_worksheet(tab_colour = wb_colour("green"))$
+      add_chartsheet(tab_colour = wb_colour("green"))$
       # colour name
-      add_worksheet(tabColour = "green")$
-      add_chartsheet(tabColour = "green")
+      add_worksheet(tab_colour = "green")$
+      add_chartsheet(tab_colour = "green")
   )
 })
 
 test_that("treat color and colour equally", {
 
   wb_color <- wb_workbook() %>%
-    wb_add_worksheet(tabColor = "green") %>%
+    wb_add_worksheet(tab_color = "green") %>%
     wb_add_fill(color = wb_color("blue")) %>%
     wb_add_border(
       dims   = "G12:H13",
@@ -43,7 +43,7 @@ test_that("treat color and colour equally", {
     )
 
   wb_colour <- wb_workbook() %>%
-    wb_add_worksheet(tabColour = "green") %>%
+    wb_add_worksheet(tab_colour = "green") %>%
     wb_add_fill(colour = wb_colour("blue")) %>%
     wb_add_border(
       dims   = "G12:H13",

tests/testthat/test-class-color.R

@@ -5,35 +5,35 @@ test_that("class wbComment works", {
 })
 
 test_that("wb_comment and create_comment are the same except for the different defaults", {
-  c1 <- create_comment("x1", author = "")
+  expect_warning(c1 <- create_comment("x1", author = ""), "deprecated")
   c1_wb <- wb_comment("x1", visible = TRUE, author = "")
   expect_equal(c1, c1_wb)
   # create_comment drops multiple widths and heights silently.
   # wb_comment errors in this case
-  expect_silent(create_comment(text = "x", author = "", width = c(1, 2)))
+  expect_warning(create_comment(text = "x", author = "", width = c(1, 2)), "deprecated")
   expect_error(wb_comment(text = "x", author = "", width = c(1, 2)), "width must be a single")
 
 })
 
 test_that("create_comment() works", {
   # error checking
-  expect_silent(create_comment("hi", width = 1))
-  expect_silent(create_comment("hi", width = 1L))
-  expect_silent(create_comment("hi", width = c(1, 2)))
-  expect_silent(create_comment("hi", width = 1:2))
+  expect_warning(create_comment("hi", width = 1), "deprecated")
+  expect_warning(create_comment("hi", width = 1L), "deprecated")
+  expect_warning(create_comment("hi", width = c(1, 2)), "deprecated")
+  expect_warning(create_comment("hi", width = 1:2), "deprecated")
   expect_error(wb_comment("hi", width = 1:2), regexp = "width must be a")
 
-  expect_silent(create_comment("hi", height = 1))
-  expect_silent(create_comment("hi", height = 1L))
-  expect_silent(create_comment("hi", height = 1:2))
+  expect_warning(create_comment("hi", height = 1), "deprecated")
+  expect_warning(create_comment("hi", height = 1L), "deprecated")
+  expect_warning(create_comment("hi", height = 1:2), "deprecated")
   expect_error(wb_comment("hi", height = 1:2))
-  expect_error(create_comment("hi", visible = NULL))
-  expect_error(create_comment("hi", visible = c(TRUE, FALSE)))
+  expect_warning(expect_error(create_comment("hi", visible = NULL)), "deprecated")
+  expect_warning(expect_error(create_comment("hi", visible = c(TRUE, FALSE))), "deprecated")
 
-  expect_error(create_comment("hi", author = 1))
-  expect_error(create_comment("hi", author = c("a", "a")))
+  expect_warning(expect_error(create_comment("hi", author = 1)), "deprecated")
+  expect_warning(expect_error(create_comment("hi", author = c("a", "a"))), "deprecated")
 
-  expect_s3_class(create_comment("Hello"), "wbComment")
+  expect_warning(expect_s3_class(create_comment("Hello"), "wbComment"), "deprecated")
 })
 
 
@@ -43,17 +43,17 @@ test_that("comments", {
   wb$add_worksheet("Sheet 1")
 
   # write comment without author
-  c1 <- create_comment(text = "this is a comment", author = "")
+  c1 <- wb_comment(text = "this is a comment", author = "")
   wb <- wb_add_comment(wb, 1, dims = "B10", comment = c1)
 
   # Write another comment with author information
-  c2 <- create_comment(text = "this is another comment", author = "Marco Polo")
+  c2 <- wb_comment(text = "this is another comment", author = "Marco Polo")
   wb <- wb_add_comment(wb, 1, dims = "C10", comment = c2)
 
   # write a styled comment with system author
   s1 <- create_font(b = "true", color = wb_colour(hex = "FFFF0000"), sz = "12")
   s2 <- create_font(color = wb_colour(hex = "FF000000"), sz = "9")
-  c3 <- create_comment(text = c("This Part Bold red\n\n", "This part black"), style = c(s1, s2))
+  c3 <- wb_comment(text = c("This Part Bold red\n\n", "This part black"), style = c(s1, s2))
 
   expect_silent(wb$add_comment(1, dims = "F1", comment = c3))
 
@@ -74,7 +74,7 @@ test_that("comments", {
   wb$add_worksheet()
 
   # write comment without author
-  c1 <- create_comment(text = "this is a comment", author = "", visible = FALSE)
+  c1 <- wb_comment(text = "this is a comment", author = "")
   wb$add_comment(dims = "B10", comment = c1)
 
   expect_silent(wb$save(tmp))
@@ -97,7 +97,7 @@ test_that("load comments", {
   unlink(tempd, recursive = TRUE)
 
   ## add a new comment to a workbook that has comments
-  c1 <- create_comment(text = "this is a comment", author = "")
+  expect_warning(c1 <- create_comment(text = "this is a comment", author = ""), "deprecated")
   expect_silent(wb$add_comment(5, dims = "B10", comment = c1))
 
   wb$save(temp)
@@ -113,7 +113,7 @@ test_that("load comments", {
 
 test_that("wb_add_comment", {
 
-  c1 <- create_comment(text = "this is a comment", author = "")
+  expect_warning(c1 <- create_comment(text = "this is a comment", author = ""), "deprecated")
 
   wb <- wb_workbook()$add_worksheet()$add_comment(dims = "A1", comment = c1)
 
@@ -155,7 +155,7 @@ test_that("wb_add_comment() works without supplying a wbComment object.", {
 
 test_that("wb_remove_comment", {
 
-  c1 <- create_comment(text = "this is a comment", author = "")
+  expect_warning(c1 <- create_comment(text = "this is a comment", author = ""), "deprecated")
   wb <- wb_workbook()$
     add_worksheet()$
     add_comment(dims = "A1", comment = c1)$
@@ -179,8 +179,10 @@ test_that("wb_remove_comment", {
 
 test_that("print comment", {
 
-  c2 <- create_comment(text = "this is another comment",
-                       author = "Marco Polo")
+  expect_warning(
+    c2 <- create_comment(text = "this is another comment",
+                       author = "Marco Polo"),
+    "deprecated")
   got <- capture_output(print(c2), print = TRUE)
   exp <- "Author: Marco Polo\nText:\n Marco Polo:\nthis is another comment\n\nStyle:\n\n\n\n\nFont name: Aptos Narrow\nFont size: 11\nFont color: #000000\n\n"
   expect_equal(got, exp)
@@ -190,7 +192,7 @@ test_that("print comment", {
 test_that("removing comment sheet works", {
 
   temp <- temp_xlsx()
-  c1 <- create_comment(text = "this is a comment", author = "")
+  expect_warning(c1 <- create_comment(text = "this is a comment", author = ""), "deprecated")
 
   wb <- wb_workbook()$
     add_worksheet("Sheet 1")$
@@ -206,7 +208,7 @@ test_that("fmt_txt in comment", {
 
   tmp <- temp_xlsx()
   txt <- fmt_txt("Hello ", bold = TRUE) + fmt_txt("World")
-  c1 <- create_comment(text = txt, author = "bla")
+  expect_warning(c1 <- create_comment(text = txt, author = "bla"), "deprecated")
 
   wb <- wb_workbook()$add_worksheet()$add_comment(dims = "B10", comment = c1)
   expect_silent(wb$save(tmp))

tests/testthat/test-class-comment.R

@@ -6,9 +6,9 @@ test_that("encode Hyperlink works", {
   formula_new <- '=HYPERLINK("#Tab_1!" & CELL("address", INDEX(C1:F1, MATCH(A1, C1:F1, 0))), "Go to the selected column")'
 
   wb <- wb_workbook()$
-    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_worksheet("Tab_1", zoom = 80, grid_lines = FALSE)$
+    add_data(x = rbind(2016:2019), dims = "C1:F1", col_names = FALSE)$
+    add_data(x = 2017, dims = "A1", col_names = FALSE)$
     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)
@@ -19,16 +19,16 @@ test_that("encode Hyperlink works", {
   got <- wb$worksheets[[1]]$sheet_data$cc["12", "f"]
   expect_equal(formula_old, got)
 
-  expect_equal(formula_new, wb_to_df(wb, colNames = FALSE, showFormula = TRUE)[1, "B"])
+  expect_equal(formula_new, wb_to_df(wb, col_names = FALSE, show_formula = TRUE)[1, "B"])
 
 })
 
 test_that("formulas with hyperlinks works", {
 
   wb <- wb_workbook()$
-    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_worksheet("Tab_1", zoom = 80, grid_lines = FALSE)$
+    add_data(dims = "C1:F1", x = rbind(2016:2019), col_names = FALSE)$
+    add_data(x = 2017, start_col = 1, start_row = 1, col_names = FALSE)$
     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)')