diff --git a/R/utils.R b/R/utils.R index 480a34c74..569e338aa 100644 --- a/R/utils.R +++ b/R/utils.R @@ -676,20 +676,29 @@ wb_dims <- function(..., select = NULL) { ) } + rows_arg <- args$rows + cols_arg <- args$cols + if (n_unnamed_args == 1 && len > 1 && !"rows" %in% nams) { message("Assuming the first unnamed argument to be `rows`.") - nams[which(nams == "")[1]] <- "rows" + rows_pos <- which(nams == "")[1] + nams[rows_pos] <- "rows" names(args) <- nams n_unnamed_args <- length(which(!nzchar(nams))) all_args_unnamed <- n_unnamed_args == len + + rows_arg <- args[[rows_pos]] } if (n_unnamed_args == 1 && len > 1 && "rows" %in% nams) { message("Assuming the first unnamed argument to be `cols`.") - nams[which(nams == "")[1]] <- "cols" + cols_pos <- which(nams == "")[1] + nams[cols_pos] <- "cols" names(args) <- nams n_unnamed_args <- length(which(!nzchar(nams))) all_args_unnamed <- n_unnamed_args == len + + cols_arg <- args[[cols_pos]] } # if 2 unnamed arguments, will be rows, cols. @@ -701,6 +710,22 @@ wb_dims <- function(..., select = NULL) { names(args) <- nams n_unnamed_args <- length(which(!nzchar(nams))) all_args_unnamed <- n_unnamed_args == len + + rows_arg <- args[[rows_pos]] + cols_arg <- args[[cols_pos]] + } + + if (!is.null(cols_arg) && (min(col2int(cols_arg)) < 1L)) { + stop("You must supply positive values to `cols`") + } + + # in case the user mixes up column and row + if (is.character(rows_arg) && !all(is_charnum(rows_arg))) { + stop("`rows` is character and contains nothing that can be interpreted as number.") + } + + if (!is.null(rows_arg) && (min(as.integer(rows_arg)) < 1L)) { + stop("You must supply positive values to `rows`.") } # Just keeping this as a safeguard @@ -709,9 +734,11 @@ wb_dims <- function(..., select = NULL) { stop("Internal error, all arguments should be named after this point.") } - if (length(args$from_col) > 1 || length(args$from_row) > 1) { + if (length(args$from_col) > 1 || length(args$from_row) > 1 || + (!is.null(args$from_row) && is.character(args$from_row) && !is_charnum(args$from_row))) { stop("from_col/from_row must be positive integers if supplied.") } + # handle from_dims if (!is.null(args$from_dims)) { if (!is.null(args$from_col) || !is.null(args$from_row)) { @@ -719,13 +746,16 @@ wb_dims <- function(..., select = NULL) { } # transform to from_row_and_col <- dims_to_rowcol(args$from_dims, as_integer = TRUE) - fcol <- col2int(from_row_and_col[[1]]) - frow <- as.integer(from_row_and_col[[2]]) + fcol <- from_row_and_col[[1]] + frow <- from_row_and_col[[2]] } else { fcol <- args$from_col %||% 1L frow <- args$from_row %||% 1L } + fcol <- col2int(fcol) + frow <- col2int(frow) + left <- args$left right <- args$right above <- args$above @@ -792,184 +822,89 @@ wb_dims <- function(..., select = NULL) { # little helper that streamlines which inputs cannot be select <- determine_select_valid(args = args, select = select) + # TODO: we warn, but fail not? Shouldn't we fail? check_wb_dims_args(args, select = select) - - args$from_col <- col2int(fcol) - args$from_row <- frow - args$from_dims <- NULL - - rows_arg <- args$rows - rows_arg <- if (is.character(rows_arg)) { - col2int(rows_arg) - } else if (!is.null(rows_arg)) { - as.integer(rows_arg) - } else if (!is.null(args$x)) { - # rows_arg <- seq_len(nrow(args$x)) - rows_arg <- NULL - } else { - 1L - } + if (!is.null(rows_arg)) rows_arg <- as.integer(rows_arg) + if (!is.null(frow)) frow <- as.integer(frow) assert_class(rows_arg, class = "integer", arg_nm = "rows", or_null = TRUE) # Checking cols (if it is a column name) cols_arg <- args$cols x_has_named_dims <- inherits(x, "data.frame") || inherits(x, "matrix") - x_has_colnames <- !is.null(colnames(x)) + if (!is.null(x)) { x <- as.data.frame(x) } - cnam_null <- is.null(args$col_names) - col_names <- args$col_names %||% x_has_named_dims + cnam <- isTRUE(args$col_names) + col_names <- if (!is.null(args$col_names)) args$col_names else x_has_named_dims + row_names <- if (!is.null(args$row_names)) args$row_names else FALSE - if (!cnam_null && !x_has_named_dims) { + if (cnam && !x_has_named_dims) { stop("Can't supply `col_names` when `x` is a vector.\n", "Transform `x` to a data.frame") } - row_names <- args$row_names %||% FALSE assert_class(col_names, "logical") assert_class(row_names, "logical") - # Find column location id if `cols` is a character and is a colname of x - if (x_has_colnames && !is.null(cols_arg)) { - is_cols_a_colname <- cols_arg %in% colnames(x) - - if (any(is_cols_a_colname)) { - cols_arg <- which(colnames(x) %in% cols_arg) - } - } - - if (!is.null(cols_arg)) { + # special condition, can be cell reference + if (is.null(x) && is.character(cols_arg)) { cols_arg <- col2int(cols_arg) - assert_class(cols_arg, class = "integer", arg_nm = "cols") - } else if (!is.null(args$x)) { - cols_arg <- NULL - } else { - cols_arg <- 1L # no more NULL for cols_arg and rows_arg if `x` is not supplied - } - - if (!is.null(cols_arg) && (min(cols_arg) < 1L)) { - stop("You must supply positive values to `cols`") - } - - if (!is.null(rows_arg) && (min(rows_arg) < 1L)) { - stop("You must supply positive values to `rows`.") } - # assess from_row / from_col - if (is.character(args$from_row)) { - frow <- col2int(args$from_row) - } else { - frow <- args$from_row %||% 1L - frow <- as.integer(frow) - } + # get base dimensions + cols_all <- if (!is.null(x)) seq_len(NCOL(x)) else cols_arg + rows_all <- if (!is.null(x)) seq_len(NROW(x)) else rows_arg - # from_row is a function of col_names, from_row and cols. - # cols_seq should start at 1 after this - # if from_row = 4, rows = 4:7, - # then frow = 4 + 4 et rows = seq_len(length(rows)) - fcol <- col2int(args$from_col) %||% 1L - # after this point, no assertion, assuming all elements to be acceptable - - # from_row / from_col = 0 only acceptable in certain cases. - if (!all(length(fcol) == 1, length(frow) == 1, fcol >= 1, frow >= 1)) { - stop("`from_col` / `from_row` must be a single positive number.") - } + # get selections in this base + cols_sel <- if (is.null(cols_arg)) cols_all else cols_arg + rows_sel <- if (is.null(rows_arg)) rows_all else rows_arg - if (select == "col_names") { - if (is.null(cols_arg) || length(cols_arg) %in% c(0, 1)) { - ncol_to_span <- ncol(x) - } else { - ncol_to_span <- cols_arg - } - nrow_to_span <- 1L - } else if (select == "row_names") { - ncol_to_span <- 1L - nrow_to_span <- nrow(x) %||% 1L - } else if (select %in% c("x", "data")) { - if (!is.null(cols_arg)) { - if (length(cols_arg) == 1) { - ncol_to_span <- length(cols_arg) - } else { - ncol_to_span <- cols_arg - } - } else { - ncol_to_span <- ncol(x) %||% 1L - } - if (!is.null(rows_arg)) { - nrow_to_span <- length(rows_arg) - } else { - nrow_to_span <- nrow(x) %||% 1L - } - - if (select == "x") { - nrow_to_span <- nrow_to_span + col_names - ncol_to_span <- ncol_to_span + row_names + # if cols is a column name from x + if (!is.null(x) && is.character(cols_sel) && any(cols_sel %in% names(x))) { + names(cols_all) <- names(x) + cols_sel <- match(cols_sel, names(cols_all)) + if (length(cols_sel) == 0) { + warning("selected column not found in `x`.") + cols_sel <- cols_all } } - # Setting frow / fcol correctly. - if (select == "row_names") { - fcol <- fcol - frow <- frow + col_names - } else if (select == "col_names") { - fcol <- fcol + row_names - frow <- frow - } else if (select %in% c("x", "data")) { - if (!is.null(cols_arg) && length(cols_arg) == 1L && all(diff(cols_arg) == 1L)) { - if (min(cols_arg) > 1) { - fcol <- fcol + min(cols_arg) - 1L # does not work with multi select - } - } else { - fcol <- fcol #+ row_names - } - - if (!is.null(rows_arg)) { # && length(rows_arg) == 1L && all(diff(rows_arg) == 1L)) { - if (min(rows_arg) > 1) { - if (all(diff(rows_arg) == 1L)) { - frow <- frow + min(rows_arg) - 1L - } else { - frow <- vapply(rows_arg, function(x) frow + min(x) - 1L, NA_real_) - } - } - } + # reduce to required length + col_span <- cols_all[cols_all %in% cols_sel] + row_span <- rows_all[rows_all %in% rows_sel] - if (select == "data") { - fcol <- fcol + row_names - frow <- frow + col_names - } + # if required add column name and row name + if (col_names) row_span <- c(max(min(row_span, 1), 1L), row_span + 1L) + if (row_names) col_span <- c(max(min(col_span, 1), 1L), col_span + 1L) - # for cases were cols and rows are vectors and row 1 is selected - # frow is a scalar and rows_arg is a vector. We need the vector. - # it's mystery, why crossing 1 is such an issue. - if (is.null(args$x) && !all(diff(rows_arg) == 1L)) { - diff <- min(frow) - min(rows_arg) - frow <- diff + rows_arg - } + # data has no column name + if (!is.null(x) && select == "data") { + if (col_names) row_span <- row_span[-1] + if (row_names) col_span <- col_span[-1] } - # Ensure we are spanning at least 1 row and 1 col - if (identical(nrow_to_span, 0L)) { - nrow_to_span <- 1L + # column name has only the column name + if (!is.null(x) && select == "col_names") { + if (col_names) row_span <- row_span[1] # else 0? + if (row_names) col_span <- col_span[-1] } - if (identical(ncol_to_span, 0L)) { - ncol_to_span <- 1L + if (!is.null(x) && select == "row_names") { + if (col_names) row_span <- row_span[-1] + if (row_names) col_span <- col_span[1] # else 0? } - if (all(diff(frow) == 1)) { - row_span <- frow + seq_len(nrow_to_span) - 1L - } else { - row_span <- frow - } + # This can happen if only from_col or from_row is supplied + if (length(row_span) == 0) row_span <- 1L + if (length(col_span) == 0) col_span <- 1L + + # frow & fcol start at 1 + row_span <- row_span + (frow - 1L) + col_span <- col_span + (fcol - 1L) - if (length(ncol_to_span) == 1) { - col_span <- fcol + seq_len(ncol_to_span) - 1L - } else { # what does this do? - col_span <- fcol + ncol_to_span - 1L - } # return single cells (A1 or A1,B1) - if (length(row_span) == 1 && (length(col_span) == 1 || any(diff(col_span) != 1L))) { + if ((length(row_span) == 1 || any(diff(row_span) != 1L)) && (length(col_span) == 1 || any(diff(col_span) != 1L))) { # A1 row_start <- row_span @@ -977,13 +912,28 @@ wb_dims <- function(..., select = NULL) { dims <- NULL - if (any(diff(col_span) != 1L)) { - for (col_start in col_span) { - tmp <- rowcol_to_dim(row_start, col_start) - dims <- c(dims, tmp) + if (any(diff(row_span) != 1L)) { + for (row_start in row_span) { + cdims <- NULL + if (any(diff(col_span) != 1L)) { + for (col_start in col_span) { + tmp <- rowcol_to_dim(row_start, col_start) + cdims <- c(cdims, tmp) + } + } else { + cdims <- rowcol_to_dim(row_start, col_span) + } + dims <- c(dims, cdims) } } else { - dims <- rowcol_to_dim(row_start, col_start) + if (any(diff(col_span) != 1L)) { + for (col_start in col_span) { + tmp <- rowcol_to_dim(row_span, col_start) + dims <- c(dims, tmp) + } + } else { + dims <- rowcol_to_dim(row_span, col_span) + } } } else { # return range "A1:A7" or "A1:A7,B1:B7" diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 21ccca1c6..73cf3edaf 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -65,8 +65,8 @@ test_that("`wb_dims()` works/errors as expected with unnamed arguments", { wb_dims(1:10, 1:12, from_row = 2), wb_dims(rows = 1:10, cols = 1:12, from_row = 2) ) - expect_warning(wb_dims("1", 2)) - expect_warning(wb_dims(from_row = "C")) + expect_warning(wb_dims("1", 2), "should not be a character") + expect_error(wb_dims(from_row = "C"), "from_col/from_row must be positive integers if supplied.") # Ambiguous / input not accepted. # This now fails, as it used not to work. (Use `wb_dims()`, `NULL`, or ) @@ -111,7 +111,7 @@ test_that("wb_dims() works when not supplying `x`.", { expect_message(out <- wb_dims(cols = 1, 2), "Assuming the .+ `rows`") expect_equal(out, "A2") # warns when trying to pass weird things - expect_warning(wb_dims(rows = "BC", cols = 1), regexp = "supply an integer") + expect_error(wb_dims(rows = "BC", cols = 1), "`rows` is character and contains nothing that can be interpreted as number.") # "`wb_dims()` works expect_equal(wb_dims(from_col = 4), "D1") expect_equal(wb_dims(from_row = 4), "A4") @@ -206,11 +206,11 @@ test_that("`wb_dims()` can select content in a nice fashion with `x`", { # Selecting a row range dims_row1_to_5 <- "B3:L7" - expect_equal(wb_dims_cars(rows = 1:5), dims_row1_to_5) + expect_equal(wb_dims(x = mtcars, from_row = 2, from_col = "B", rows = 1:5), dims_row1_to_5) # Select a row range with the names of `x` dims_row1_to_5_and_names <- "B2:L7" - expect_equal(wb_dims_cars(rows = 1:5, select = "x"), dims_row1_to_5_and_names) + expect_equal(wb_dims(x = mtcars, from_row = 2, from_col = "B", rows = 1:5, select = "x"), dims_row1_to_5_and_names) }) test_that("wb_dims can select multiple columns and rows", { @@ -247,7 +247,7 @@ test_that("wb_dims can select multiple columns and rows", { got <- wb_dims(x = mtcars, rows = c(2, 4:5)) expect_equal(exp, got) - exp <- "B3:B3,D3:D3,B5:B5,D5:D5,B6:B6,D6:D6" + exp <- "B3,D3,B5,D5,B6,D6" got <- wb_dims(x = mtcars, cols = c(2, 4), rows = c(2, 4:5)) expect_equal(exp, got) @@ -286,8 +286,6 @@ test_that("`wb_dims()` works when Supplying an object `x`.", { # select rows and columns work expect_equal(wb_dims(x = mtcars, rows = 2:10, cols = "cyl"), "B3:B11") - - expect_equal(wb_dims(rows = 1 + seq_len(nrow(mtcars)), cols = 4), "D2:D33") out_hp <- wb_dims(x = mtcars, cols = "hp") # , "col name = 'hp' to `cols = 4`") expect_equal(out_hp, "D2:D33") @@ -307,7 +305,7 @@ test_that("`wb_dims()` works when Supplying an object `x`.", { # using non-existing character column doesn't work expect_error(wb_dims(x = mtcars, cols = "A"), "`cols` must be an integer or an existing column name of `x`.") expect_equal(wb_dims(x = mtcars, cols = c("hp", "vs")), "D2:D33,H2:H33") - expect_error(expect_warning(wb_dims(x = mtcars, rows = "hp")), "[Uu]se `cols` instead.") + expect_error(expect_warning(wb_dims(x = mtcars, rows = "hp")), "`rows` is character and contains nothing that can be interpreted as number.") # Access only row / col name expect_no_message(wb_dims(x = mtcars, select = "col_names")) # to write without column names, specify `from_row = 0` (or -1 of what you wanted) @@ -389,36 +387,67 @@ test_that("wb_dims() handles `from_dims`", { test_that("wb_dims() corner cases work", { - exp <- "B2:B2,B30:B30" + exp <- "B2,B30" got <- wb_dims(rows = c(2, 30), cols = 2) # expect B2,B30 expect_equal(exp, got) - exp <- "B1:B1,B30:B30" + exp <- "B1,B30" got <- wb_dims(rows = c(1, 30), cols = 2) # expect B1,B30 expect_equal(exp, got) - exp <- "B11:B11,B40:B40" + exp <- "B11,B40" got <- wb_dims(rows = c(1, 30), cols = 2, from_row = 11) expect_equal(exp, got) - exp <- "B2:B2,D2:D2,B30:B30,D30:D30" + exp <- "B2,D2,B30,D30" got <- wb_dims(rows = c(2, 30), cols = c(2, 4)) # expect B2,D2,B30,D30 expect_equal(exp, got) - exp <- "B1:B1,D1:D1,B30:B30,D30:D30" + exp <- "B1,D1,B30,D30" got <- wb_dims(rows = c(1, 30), cols = c(2, 4)) # expect B1,D1,B30,D30 expect_equal(exp, got) - exp <- "B2:B2,D2:D2,B1:B1,D1:D1" + exp <- "B2,D2,B1,D1" got <- wb_dims(rows = c(2, 1), cols = c(2, 4)) # expect B2,D2,B1,D1 expect_equal(exp, got) - exp <- "B5:B5,D5:D5,B1:B1,D1:D1" + exp <- "B5,D5,B1,D1" got <- wb_dims(rows = c(5, 1), cols = c(2, 4)) # expect B5,D5,B1,D1 expect_equal(exp, got) }) +test_that("row and col selection works", { + + # equal with sample cols + expect_equal( + wb_dims(x = mtcars, cols = c(1, 4:7, 9)), + wb_dims(rows = seq_len(nrow(mtcars)) + 1L, cols = c(1, 4:7, 9)) + ) + + # unequeal with sample rows + expect_equal( + wb_dims(x = mtcars, rows = which(mtcars$cyl == 6)), + wb_dims(rows = which(mtcars$cyl == 6) + 1L, cols = seq_along(mtcars)) + ) + + expect_equal( + wb_dims(x = mtcars, rows = which(mtcars$cyl == 6), from_row = 4), + wb_dims(rows = which(mtcars$cyl == 6) + 1L, cols = seq_along(mtcars), from_row = 4) + ) + + expect_equal( + wb_dims(x = mtcars, rows = which(mtcars$cyl == 6), from_col = 6), + wb_dims(rows = which(mtcars$cyl == 6) + 1L, cols = seq_along(mtcars), from_col = 6) + ) + + expect_equal( + wb_dims(x = mtcars, cols = c(1, 4:7, 9), row_names = TRUE), + wb_dims(rows = seq_len(nrow(mtcars)) + 1L, cols = c(1, 4:7, 9), from_col = 2) + ) + +}) + test_that("create_char_dataframe", { exp <- data.frame(x1 = rep("", 5), z1 = rep("", 5), stringsAsFactors = FALSE)