Make wb_get_named_regions() a wrapper. (#764)

Co-authored-by: Jan Marvin Garbuszus <jan.garbuszus@ruhr-uni-bochum.de>

NEWS.md

@@ -49,8 +49,7 @@ They will continue to work for some time, but changing to newer functions is rec
 
 ## Internal changes
 
-* `wb_get_active_sheet()`, `wb_set_active_sheet()`, `wb_get_selected()` and `wb_set_selected()` are now wrapper functions. [735](https://github.com/JanMarvin/openxlsx2/pull/735)
-
+* `wb_get_active_sheet()`, `wb_set_active_sheet()`, `wb_get_selected()` and `wb_set_selected()`, `wb_get_named_regions()` are now wrapper functions. [735](https://github.com/JanMarvin/openxlsx2/pull/735)
 
 
 ***************************************************************************

R/class-workbook-wrappers.R

@@ -1723,8 +1723,7 @@ wb_set_order <- function(wb, sheets) {
 #'
 #' ## delete one
 #' wb$remove_named_region(name = "iris2")
-#' wb_get_named_regions(wb)
-#'
+#' wb$get_named_regions()
 #' ## read named regions
 #' df <- wb_to_df(wb, named_region = "iris")
 #' head(df)

R/class-workbook.R

@@ -2740,7 +2740,7 @@ wbWorkbook <- R6::R6Class(
 
         ## rename defined names
         if (length(self$workbook$definedNames)) {
-          ind <- wb_get_named_regions(self)
+          ind <- self$get_named_regions()
           # TODO why is the order switched?
           ind <- ind[order(as.integer(rownames(ind))), ]
           ind <- ind$sheets == old
@@ -5722,6 +5722,33 @@ wbWorkbook <- R6::R6Class(
       invisible(self)
     },
 
+    #' @description get named regions in a workbook
+    #' @param tables Return tables as well?
+    #' @param x Not used.
+    #' @return A `data.frame` of named regions
+    get_named_regions = function(tables = FALSE, x = NULL) {
+      if (!is.null(x)) {
+        stop("x should not be provided to get_named_regions.", call. = FALSE)
+      }
+      z <- NULL
+
+      if (length(self$workbook$definedNames)) {
+        z <- get_nr_from_definedName(self)
+      }
+
+      if (tables && !is.null(self$tables)) {
+        tb <- get_named_regions_tab(self)
+
+        if (is.null(z)) {
+          z <- tb
+        } else {
+          z <- merge(z, tb, all = TRUE, sort = FALSE)
+        }
+
+      }
+
+      z
+    },
     #' @description remove a named region
     #' @param sheet sheet
     #' @param name name

R/get-named-regions.R

@@ -40,7 +40,7 @@ get_nr_from_definedName <- function(wb) {
 #' @param wb a workbook
 #' @returns a data frame in named_region format
 #' @noRd
-wb_get_named_regions_tab <- function(wb) {
+get_named_regions_tab <- function(wb) {
   data.frame(
     #localSheetId is not always available
     name = wb$tables$tab_nam,
@@ -54,12 +54,15 @@ wb_get_named_regions_tab <- function(wb) {
   )
 }
 
-#' Get named regions in a workbook or an xlsx file
+#' Get named regions in a workbook
 #'
 #' @returns A vector of named regions in `x`.
-#' @param x An xlsx file or a`wbWorkbook` object
+#' @param wb A `wbWorkbook` object
 #' @param tables Should data tables be included in the result?
+#' @param x deprecated. Use `wb`. For Excel input use [wb_load()] to first load
+#'   the xlsx file as a workbook.
 #' @seealso [wb_add_named_region()], [wb_get_tables()]
+#' @returns A data frame with the all named regions in `wb`. Or `NULL`, if none are found.
 #' @export
 #' @examples
 #' wb <- wb_workbook()
@@ -71,39 +74,50 @@ wb_get_named_regions_tab <- function(wb) {
 #'   name = "iris",
 #'   dims = wb_dims(x = iris)
 #' )$add_data(sheet = 1, x = iris, name = "iris2", start_col = 10)
+#' ## From Workbook object
+#' wb_get_named_regions(wb)
+#' # Use this info to extract the data frame
+#' df <- wb$to_df(named_region = "iris2")
+#' head(df)
+#'
+#' # Extract tables and named regions
+#' wb$add_worksheet()$add_data_table(x = iris)
+#'
+#' wb$get_named_regions(tables = TRUE)
 #'
+#' # Extract named regions from a file
 #' out_file <- temp_xlsx()
 #' wb_save(wb, out_file, overwrite = TRUE)
 #'
-#' ## see named regions
-#' wb_get_named_regions(wb) ## From Workbook object
-#' wb_get_named_regions(out_file) ## From xlsx file
+#' # Load the file as a workbook first, then get named regions.
+#' wb1 <- wb_load(out_file)
+#' wb1$get_named_regions()
 #'
-#' df <- read_xlsx(out_file, named_region = "iris2")
-#' head(df)
-wb_get_named_regions <- function(x, tables = FALSE) {
-  if (inherits(x, "wbWorkbook")) {
-    wb <- x
-  } else {
-    wb <- wb_load(x)
-  }
+wb_get_named_regions <- function(wb, tables = FALSE, x = NULL) {
+  # TODO merge this doc with wb_add_named_region
+  if (!is.null(x)) {
+    # Will only show up if the user named `x`
+    .Deprecated("wb", old = "x", msg = "Use `wb` instead in `wb_get_named_regions()`")
 
-  z <- NULL
+    if (!missing(wb)) {
+      # if a user tries to provide both x and wb.
+      stop("x is a deprecated argument. Use wb instead. can't be supplied. Use `wb` only.")
+    }
 
-  if (length(wb$workbook$definedNames)) {
-    z <- get_nr_from_definedName(wb)
+    wb <- x
   }
 
-  if (tables && !is.null(wb$tables)) {
-    tb <- wb_get_named_regions_tab(wb)
-
-    if (is.null(z)) {
-      z <- tb
-    } else {
-      z <- merge(z, tb, all = TRUE, sort = FALSE)
+  if (!inherits(wb, "wbWorkbook")) {
+    if (getOption("openxlsx2.soon_deprecated", default = FALSE)) {
+      warning(
+        "Using `wb_get_named_regions()` on an xlsx file is deprecated.\n",
+        "Use `wb_load(file)$get_named_regions()` instead.",
+        call. = FALSE
+        )
     }
-
+    wb <- wb_load(wb)
   }
 
-  z
+  assert_workbook(wb)
+  wb$get_named_regions(tables = tables)
 }

R/xl_open.R

@@ -37,14 +37,14 @@
 #' }
 #' @export
 xl_open <- function(x, interactive = NA) {
-  # one of the only functions like wb_get_named_regions() to accept a wbWorkbook or a file
+  # The only function to accept a workbook or a file.
   UseMethod("xl_open")
 }
 
 #' @rdname xl_open
 #' @export
 xl_open.wbWorkbook <- function(x, interactive = NA) {
-  stopifnot(R6::is.R6(x))
+  assert_workbook(x)
   has_macros <- isTRUE(length(x$vbaProject) > 0)
   xl_open(x$clone()$save(temp_xlsx(macros = has_macros))$path, interactive = interactive)
 }

tests/testthat/test-class-workbook-wrappers.R

@@ -258,12 +258,13 @@ test_that("wb_set_grid_lines() is a wrapper", {
 
 # wb_add_named_region(), wb_remove_named_region() -------------------------
 
-test_that("wb_add_named_region(), wb_remove_named_region() are wrappers", {
+test_that("wb_add_named_region(), wb_remove_named_region() wb_get_named_regions() are wrappers", {
   wb <- wb_workbook()$add_worksheet("a")
   params <- list(sheet = 1, dims = "A1", name = "cool")
   expect_wrapper("add_named_region", wb = wb, params = params)
   # now add the named region so that we can remove it
   wb$add_named_region(sheet = 1, dims = "A1", name = "cool")
+  expect_wrapper("get_named_regions", wb = wb)
   expect_wrapper("remove_named_region", wb = wb, params = list(name = "cool"))
 })
 

tests/testthat/test-named_regions.R

@@ -27,7 +27,10 @@ test_that("Maintaining Named Regions on Load", {
   out_file <- temp_xlsx()
   wb_save(wb, out_file, overwrite = TRUE)
 
-  expect_equal(object = wb_get_named_regions(wb), expected = wb_get_named_regions(out_file))
+  expect_equal(
+    object = wb_get_named_regions(wb),
+    expected = wb_get_named_regions(wb_load(out_file))
+    )
 
   df1 <- read_xlsx(wb, namedRegion = "iris")
   df2 <- read_xlsx(out_file, namedRegion = "iris")
@@ -132,7 +135,7 @@ test_that("Load names from an Excel file with funky non-region names", {
   )
   expect_equal(dn$coords, c(rep("", 26), "B3", "B4", "B4", "B3"))
 
-  dn2 <- wb_get_named_regions(filename)
+  dn2 <- wb_get_named_regions(wb_load(filename))
   expect_equal(dn, dn2)
 })
 
@@ -324,7 +327,8 @@ test_that("Matching Substrings breaks reading named regions", {
   expect_equal(r1$coords, c("C12:G18", "I3:M6", "E24:P30", "O12:Z15"))
   expect_equal(r1$name, c("t", "t1", "t2", "t22"))
 
-  r2 <- wb_get_named_regions(temp_file)
+  wb2 <- wb_load(temp_file)
+  r2 <- wb_get_named_regions(wb2)
   expect_equal(r2$sheets, c("table", "table", "table2", "table2"))
   expect_equal(r1$coords, c("C12:G18", "I3:M6", "E24:P30", "O12:Z15"))
   expect_equal(r2$name, c("t", "t1", "t2", "t22"))
@@ -450,7 +454,29 @@ test_that("load table", {
 
 })
 
-test_that("get_named_regions is deprecated", {
+test_that("wb_named_regions() is not too noisy in its deprecation. (#764)", {
+  wb <- wb_workbook()$add_worksheet()
+  temp_file <- temp_xlsx()
+  wb$save(temp_file)
+  # unacceptable input only possible after 1.0
+  expect_error(expect_warning(wb_get_named_regions(temp_file, x = 1)))
+
+  opt_deprecation <- getOption("openxlsx2.soon_deprecated")
+  options("openxlsx2.soon_deprecated" = FALSE)
+  wb <- wb_workbook()$add_worksheet()
+  temp_file <- temp_xlsx()
+  wb$save(temp_file)
+  expect_no_warning(wb_get_named_regions(temp_file))
+  expect_warning(wb_get_named_regions(x = temp_file))
+
+  options("openxlsx2.soon_deprecated" = TRUE)
+  expect_warning(wb_get_named_regions(temp_file))
+  expect_warning(expect_warning(wb_get_named_regions(x = temp_file)))
+
+  options("openxlsx2.soon_deprecated" = opt_deprecation)
+})
+
+test_that("named regions work.", {
 
   wb <- wb_workbook()$add_worksheet()$add_named_region(
     name = "named_region",

tests/testthat/test-standardize.R

@@ -21,6 +21,8 @@ test_that("standardize works", {
 
 test_that("deprecation warning works", {
 
+  opt_deprecation <- getOption("openxlsx2.soon_deprecated")
+
   xlsxFile <- system.file("extdata", "openxlsx2_example.xlsx", package = "openxlsx2")
   wb1 <- wb_load(xlsxFile)
 
@@ -30,6 +32,7 @@ test_that("deprecation warning works", {
     wb_to_df(wb1, colNames = TRUE),
     "Found camelCase arguments in code. These will be deprecated in the next major release. Consider using: col_names"
   )
+  # Do not alter the global state.
+  options("openxlsx2.soon_deprecated" = opt_deprecation)
 
-  options("openxlsx2.soon_deprecated" = FALSE)
 })