[dims] previously non-consecutive dims could get lost, if rows and co…

…lumns were entirely omitted. This could create a cc object that was to short. fixes #1014 (#1015)

NEWS.md

@@ -6,6 +6,10 @@
 
 * Silence a warning triggered by a folder called `"[trash]"`. [1012](https://github.com/JanMarvin/openxlsx2/pull/1012)
 
+## Fixes
+
+* Fixed an issue with non consecutive dims, where columns or rows were silently dropped. [1015](https://github.com/JanMarvin/openxlsx2/pull/1015)
+
 
 ***************************************************************************
 

R/class-workbook.R

@@ -9151,7 +9151,7 @@ wbWorkbook <- R6::R6Class(
         if (length(need_cells) == 1 && grepl(":|;", need_cells))
           need_cells <- dims_to_dataframe(dims, fill = TRUE)
 
-        exp_cells <- unname(unlist(need_cells))
+        exp_cells <- unname(unlist(need_cells[need_cells != ""]))
         got_cells <- self$worksheets[[sheet]]$sheet_data$cc$r
 
         # initialize cell

R/read.R

@@ -249,7 +249,7 @@ wb_to_df <- function(
   xlsx_posix_style <- style_is_posix(wb$styles_mgr$styles$cellXfs, numfmt_posix)
 
   # create temporary data frame. hard copy required
-  z  <- dims_to_dataframe(dims)
+  z  <- dims_to_dataframe(dims, empty_rm = TRUE)
   tt <- copy(z)
 
   keep_cols <- colnames(z)
@@ -738,7 +738,7 @@ wb_data <- function(wb, sheet = current_sheet(), dims, ...) {
   }
 
   z <- wb_to_df(wb, sheet, dims = dims, ...)
-  attr(z, "dims")  <- dims_to_dataframe(dims, fill = TRUE)
+  attr(z, "dims")  <- dims_to_dataframe(dims, fill = TRUE, empty_rm = TRUE)
   attr(z, "sheet") <- sheetname
 
   class(z) <- c("wb_data", "data.frame")

R/wb_functions.R

@@ -2,11 +2,12 @@
 #'
 #' @param dims Character vector of expected dimension.
 #' @param fill If `TRUE`, fills the dataframe with variables
+#' @param empty_rm Logical if empty columns and rows should be included
 #' @examples
 #' dims_to_dataframe("A1:B2")
 #' @keywords internal
 #' @export
-dims_to_dataframe <- function(dims, fill = FALSE) {
+dims_to_dataframe <- function(dims, fill = FALSE, empty_rm = FALSE) {
 
   has_dim_sep <- FALSE
   if (any(grepl(";", dims))) {
@@ -51,8 +52,17 @@ dims_to_dataframe <- function(dims, fill = FALSE) {
   }
 
   if (has_dim_sep) {
-    cols_out <- int2col(sort(col2int(cols_out)))
-    rows_out <- sort(rows_out)
+    if (empty_rm) {
+      cols_out <- int2col(sort(col2int(cols_out)))
+      rows_out <- sort(rows_out)
+    } else {
+      # somehow we have to make sure that all columns are covered
+      col_ints <- col2int(cols_out)
+      cols_out <- int2col(seq.int(from = min(col_ints), to = max(col_ints)))
+
+      row_ints <- rows_out
+      rows_out <- seq.int(from = min(row_ints), to = max(row_ints))
+    }
   }
 
   dims_to_df(

R/write.R

@@ -368,6 +368,9 @@ write_data2 <- function(
   # create <rows ...>
   want_rows <- as.integer(dims_to_rowcol(dims)[[2]])
   rows_attr <- empty_row_attr(n = length(want_rows))
+  # number of rows might differ
+  if (enforce) rows_attr <- empty_row_attr(n = nrow(rtyp))
+
   rows_attr$r <- rownames(rtyp)
 
   # original cc data frame

tests/testthat/test-wb_functions.R

@@ -175,9 +175,15 @@ test_that("dims_to_dataframe", {
     class = "data.frame"
   )
   got <- dims_to_dataframe("A1:A2;C1:C2", fill = TRUE)
+  expect_equal(exp, got[c("A", "C")])
+
+  got <- dims_to_dataframe("A1;A2,C1;C2", fill = TRUE)
+  expect_equal(exp, got[c("A", "C")])
+
+  got <- dims_to_dataframe("A1:A2;C1:C2", fill = TRUE, empty_rm = TRUE)
   expect_equal(exp, got)
 
-  got <- dims_to_dataframe("A1;A2;C1;C2", fill = TRUE)
+  got <- dims_to_dataframe("A1;A2,C1;C2", fill = TRUE, empty_rm = TRUE)
   expect_equal(exp, got)
 
   exp <- list(c("A", "B"), "1")
@@ -419,3 +425,57 @@ test_that("dims with separator work", {
   expect_equal(exp, got)
 
 })
+
+test_that("improve non consecutive dims", {
+
+  # dims <- "B7:B9,C6:C10,D5:D11,E5:E12,F6:F13,G7:G14,H6:H13,I5:I12,J5:J11,K6:K10,L7:L9"
+  dims <- "B7:B9,C6:C10,D5:D11,E5:E12,G7:G14,H6:H13,I5:I12,J5:J11,K6:K10,L7:L9"
+
+  wb1 <- wb_workbook()$
+    add_worksheet(grid_lines = FALSE)$
+    set_col_widths(cols = "A:M", widths = 2)$
+    add_fill(dims = dims, color = wb_color("red"))
+
+  wb2 <- wb_workbook()$
+    add_worksheet(grid_lines = FALSE)$
+    set_col_widths(cols = "A:M", widths = 2)$
+    add_data(x = "", dims = "B5:L14")$
+    add_fill(dims = dims, color = wb_color("red"))
+
+  expect_true(wb1$worksheets[[1]]$dimension == wb2$worksheets[[1]]$dimension)
+
+  # TODO might want to exclude the empty cells here
+  exp <- dims_to_dataframe(dims, fill = TRUE)
+  exp <- unname(unlist(exp[exp != ""]))
+  got <- wb1$worksheets[[1]]$sheet_data$cc$r[wb1$worksheets[[1]]$sheet_data$cc$c_s != ""]
+  expect_true(all(exp %in% got))
+
+  got <- wb2$worksheets[[1]]$sheet_data$cc$r[wb2$worksheets[[1]]$sheet_data$cc$c_s != ""]
+  expect_true(all(exp %in% got))
+
+  ### Test rowwise
+  # dims <- "D5:E5,I5:J5,C6:F6,H6:K6,B7:L9,C10:K10,D11:J11,E12:I12,F13:H13,G14"
+  dims <- "D5:E5,I5:J5,B7:L9,C10:K10,E12:I12,F13:H13,G14"
+
+  wb3 <- wb_workbook()$
+    add_worksheet(grid_lines = FALSE)$
+    set_col_widths(cols = "A:M", widths = 2)$
+    add_fill(dims = dims, color = wb_color("red"))
+
+  wb4 <- wb_workbook()$
+    add_worksheet(grid_lines = FALSE)$
+    set_col_widths(cols = "A:M", widths = 2)$
+    add_data(x = "", dims = "B5:L14")$
+    add_fill(dims = dims, color = wb_color("red"))
+
+  expect_true(wb3$worksheets[[1]]$dimension == wb4$worksheets[[1]]$dimension)
+
+  exp <- dims_to_dataframe(dims, fill = TRUE)
+  exp <- unname(unlist(exp[exp != ""]))
+  got <- wb3$worksheets[[1]]$sheet_data$cc$r[wb3$worksheets[[1]]$sheet_data$cc$c_s != ""]
+  expect_true(all(exp %in% got))
+
+  got <- wb4$worksheets[[1]]$sheet_data$cc$r[wb4$worksheets[[1]]$sheet_data$cc$c_s != ""]
+  expect_true(all(exp %in% got))
+
+})