diff --git a/NEWS.md b/NEWS.md index cb0fc954..bf9b8c70 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,8 @@ ## Fixes * `write.xlsx()` now successfully passes `withFilter` ([#151](https://github.com/ycphs/openxlsx/issues/151)) +* code clean up PR [#168](https://github.com/ycphs/openxlsx/pull/168) +* removal of unused variables PR [#168](https://github.com/ycphs/openxlsx/pull/168) ## New features @@ -11,11 +13,6 @@ * These can be set with the `summaryCol` and `summaryRow` arguments in `pageSetup()`. * activeSheet allows to set and get the active (displayed) sheet of a worbook. -## New Features - -* Added ability to change positioning of summary columns and rows. - * These can be set with the `summaryCol` and `summaryRow` arguments in `pageSetup()`. - # openxlsx 4.2.3 ## New Features diff --git a/R/CommentClass.R b/R/CommentClass.R index 11dc1d38..c3f9adbe 100644 --- a/R/CommentClass.R +++ b/R/CommentClass.R @@ -121,7 +121,7 @@ createComment <- function(comment, width <- round(width) height <- round(height) - n <- length(comment) + # n <- length(comment) variable not used author <- author[1] visible <- visible[1] diff --git a/R/WorkbookClass.R b/R/WorkbookClass.R index 71194d3c..55544cd4 100644 --- a/R/WorkbookClass.R +++ b/R/WorkbookClass.R @@ -1996,9 +1996,9 @@ Workbook$methods( xlworksheetsDir, xlworksheetsRelsDir) { ## write worksheets - nSheets <- length(worksheets) - - for (i in 1:nSheets) { + # nSheets <- length(worksheets) + + for (i in seq_along(worksheets)) { ## Write drawing i (will always exist) skip those that are empty if (any(drawings[[i]] != "")) { write_file( @@ -2624,7 +2624,7 @@ Workbook$methods( ## Increment priority of conditional formatting rule if (length(worksheets[[sheet]]$conditionalFormatting) > 0) { - for (i in length(worksheets[[sheet]]$conditionalFormatting):1) { + for (i in rev(seq_along(worksheets[[sheet]]$conditionalFormatting))) { priority <- regmatches( worksheets[[sheet]]$conditionalFormatting[[i]], @@ -2635,16 +2635,16 @@ Workbook$methods( ) ) priority_new <- as.integer(priority) + 1L - + priority_pattern <- sprintf('priority="%s"', priority) priority_new <- sprintf('priority="%s"', priority_new) - + ## now replace worksheets[[sheet]]$conditionalFormatting[[i]] <<- gsub(priority_pattern, - priority_new, - worksheets[[sheet]]$conditionalFormatting[[i]], - fixed = TRUE + priority_new, + worksheets[[sheet]]$conditionalFormatting[[i]], + fixed = TRUE ) } } @@ -3491,7 +3491,7 @@ Workbook$methods( ## ********** Assume all styleObjects cells have one a single worksheet ********** ## Loop through existing styleObjects - newInds <- 1:length(rows) + newInds <- seq_along(rows) keepStyle <- rep(TRUE, nStyles) for (i in 1:nStyles) { if (sheet == styleObjects[[i]]$sheet) { @@ -3695,7 +3695,7 @@ Workbook$methods( ## loop through existing tables checking if any over lap with new table - for (i in 1:length(exTable)) { + for (i in seq_along(exTable)) { existing_cols <- cols[[i]] existing_rows <- rows[[i]] @@ -17850,7 +17850,7 @@ Workbook$methods( ## Increment priority of conditional formatting rule if (length((worksheets[[sheet]]$conditionalFormatting)) > 0) { - for (i in length(worksheets[[sheet]]$conditionalFormatting):1) { + for (i in rev(seq_along(worksheets[[sheet]]$conditionalFormatting))) { worksheets[[sheet]]$conditionalFormatting[[i]] <<- gsub('(?<=priority=")[0-9]+', i + 1L, @@ -17962,7 +17962,7 @@ Workbook$methods( formatCodes <- sapply(numFmts, getAttr, tag = 'formatCode="', USE.NAMES = FALSE) numFmts <- - lapply(1:length(numFmts), function(i) { + lapply(seq_along(numFmts), function(i) { list("numFmtId" = numFmtsIds[[i]], "formatCode" = formatCodes[[i]]) }) numFmtFlag <- TRUE @@ -18016,7 +18016,7 @@ Workbook$methods( regmatches(xfAttrs, regexpr('(?<=").*?(?=")', xfAttrs, perl = TRUE)) }) - for (i in 1:length(xf)) { + for (i in seq_along(xf)) { names(xfVals[[i]]) <- xfNames[[i]] } diff --git a/R/loadWorkbook.R b/R/loadWorkbook.R index b3b45884..b549e1a4 100644 --- a/R/loadWorkbook.R +++ b/R/loadWorkbook.R @@ -182,7 +182,7 @@ loadWorkbook <- function(file, xlsxFile = NULL, isUnzipped = FALSE) { j <- 1 for (i in seq_along(sheetrId)) { if (is_chart_sheet[i]) { - count <- 0 + # count <- 0 variable not used txt <- paste(readUTF8(chartSheetsXML[j]), collapse = "") zoom <- regmatches(txt, regexpr('(?<=zoomScale=")[0-9]+', txt, perl = TRUE)) @@ -930,8 +930,8 @@ loadWorkbook <- function(file, xlsxFile = NULL, isUnzipped = FALSE) { ## pivot tables if (length(pivotTableXML) > 0) { - pivotTableJ <- lapply(xml, function(x) as.integer(regmatches(x, regexpr("(?<=pivotTable)[0-9]+(?=\\.xml)", x, perl = TRUE)))) - sheetWithPivot <- which(sapply(pivotTableJ, length) > 0) + # pivotTableJ <- lapply(xml, function(x) as.integer(regmatches(x, regexpr("(?<=pivotTable)[0-9]+(?=\\.xml)", x, perl = TRUE)))) variable not used + # sheetWithPivot <- which(sapply(pivotTableJ, length) > 0) variable not used pivotRels <- lapply(xml, function(x) { y <- x[grepl("pivotTable", x)] diff --git a/R/openxlsxCoerce.R b/R/openxlsxCoerce.R index e51e5b7b..c98b87fb 100644 --- a/R/openxlsxCoerce.R +++ b/R/openxlsxCoerce.R @@ -203,7 +203,7 @@ openxlsxCoerce.coxph <- function(x, rowNames) { openxlsxCoerce.summary.coxph <- function(x, rowNames) { coef <- x$coefficients ci <- x$conf.int - nvars <- nrow(coef) + # nvars <- nrow(coef) variable not used tmp <- cbind( coef[, -ncol(coef), drop = FALSE], # p later diff --git a/R/workbook_column_widths.R b/R/workbook_column_widths.R index 0805537a..0e9cdb73 100644 --- a/R/workbook_column_widths.R +++ b/R/workbook_column_widths.R @@ -91,7 +91,7 @@ Workbook$methods(setColWidths = function(sheet) { sd <- worksheets[[sheet]]$sheet_data if (length(merge_cols) > 0) { - all_merged_cells <- lapply(1:length(merge_cols), function(i) { + all_merged_cells <- lapply(seq_along(merge_cols), function(i) { expand.grid( "rows" = min(merge_rows[[i]]):max(merge_rows[[i]]), "cols" = min(merge_cols[[i]]):max(merge_cols[[i]]) diff --git a/R/workbook_read_workbook.R b/R/workbook_read_workbook.R index aa4b61d7..6507688d 100644 --- a/R/workbook_read_workbook.R +++ b/R/workbook_read_workbook.R @@ -309,7 +309,7 @@ read.xlsx.Workbook <- function(xlsxFile, })) - dateIds <- NULL + # dateIds <- NULL variable not used if (length(format_codes) > 0) { ## this regex defines what "looks" like a date diff --git a/R/workbook_write_data.R b/R/workbook_write_data.R index b5040587..4a1c1b2a 100644 --- a/R/workbook_write_data.R +++ b/R/workbook_write_data.R @@ -58,7 +58,7 @@ Workbook$methods(writeData = function(df, sheet, startRow, startCol, colNames, c offSet <- lapply(t, parseOffset) offSet <- lapply(offSet, function(x) ifelse(is.na(x), 0, x)) - for (i in 1:length(pInds)) { + for (i in seq_along(pInds)) { df[[pInds[i]]] <- as.numeric(as.POSIXct(df[[pInds[i]]])) / 86400 + origin + offSet[[i]] } } @@ -227,7 +227,7 @@ Workbook$methods(writeData = function(df, sheet, startRow, startCol, colNames, c } ## this is text to display instead of hyperlink ## create hyperlink objects - newhl <- lapply(1:length(hyperlink_inds), function(i) { + newhl <- lapply(seq_along(hyperlink_inds), function(i) { Hyperlink$new(ref = hyperlink_refs[i], target = targets[i], location = NULL, display = NULL, is_external = TRUE) }) diff --git a/R/worksheet_class.R b/R/worksheet_class.R index 19831368..e0f43600 100644 --- a/R/worksheet_class.R +++ b/R/worksheet_class.R @@ -170,8 +170,8 @@ WorkSheet$methods(get_post_sheet_data = function() { if (length(hyperlinks) > 0) { - h_inds <- paste0(1:length(hyperlinks), "h") - xml <- paste(xml, paste("", paste(sapply(1:length(h_inds), function(i) hyperlinks[[i]]$to_xml(h_inds[i])), collapse = ""), ""), collapse = "") + h_inds <- paste0(seq_along(hyperlinks), "h") + xml <- paste(xml, paste("", paste(sapply(seq_along(h_inds), function(i) hyperlinks[[i]]$to_xml(h_inds[i])), collapse = ""), ""), collapse = "") } diff --git a/R/wrappers.R b/R/wrappers.R index e5b401dc..820bf6ea 100644 --- a/R/wrappers.R +++ b/R/wrappers.R @@ -594,7 +594,8 @@ convertFromExcelRef <- function(col) { if (any(charFlag)) { col[charFlag] <- gsub("[0-9]", "", col[charFlag]) d <- lapply(strsplit(col[charFlag], split = ""), function(x) match(rev(x), LETTERS)) - col[charFlag] <- unlist(lapply(1:length(d), function(i) sum(d[[i]] * (26^(0:(length(d[[i]]) - 1L)))))) + col[charFlag] <- unlist(lapply(seq_along(d), function(i) sum(d[[i]] * (26^( + seq_along(d[[i]]) - 1))))) } col[!charFlag] <- as.integer(col[!charFlag]) @@ -863,7 +864,7 @@ createStyle <- function(fontName = NULL, ## background fill if (is.null(bgFill)) { - bgFillList <- NULL + # bgFillList <- NULL variable not used } else { bgFill <- validateColour(bgFill, "Invalid bgFill colour") style$fill <- append(style$fill, list(fillBg = list("rgb" = bgFill))) @@ -871,7 +872,7 @@ createStyle <- function(fontName = NULL, ## foreground fill if (is.null(fgFill)) { - fgFillList <- NULL + # fgFillList <- NULL variable not used } else { fgFill <- validateColour(fgFill, "Invalid fgFill colour") style$fill <- append(style$fill, list(fillFg = list(rgb = fgFill))) @@ -2343,7 +2344,7 @@ protectWorksheet <- function(wb, sheet, protect = TRUE, password = NULL, } sheet <- wb$validateSheet(sheet) - xml <- wb$worksheets[[sheet]]$sheetProtection + # xml <- wb$worksheets[[sheet]]$sheetProtection variable not used props <- c() @@ -3020,8 +3021,9 @@ setHeader <- function(wb, text, position = "center") { stop("Text argument must be a character vector of length 1") } - sheet <- wb$validateSheet(1) - wb$headFoot$text[wb$headFoot$pos == position & wb$headFoot$head == "head"] <- as.character(text) + # sheet <- wb$validateSheet(1) variable not used + wb$headFoot$text[wb$headFoot$pos == position & wb$headFoot$head == "head"] <- + as.character(text) } @@ -3067,7 +3069,7 @@ setFooter <- function(wb, text, position = "center") { stop("Text argument must be a character vector of length 1") } - sheet <- wb$validateSheet(1) + # sheet <- wb$validateSheet(1) variable not used wb$headFoot$text[wb$headFoot$pos == position & wb$headFoot$head == "foot"] <- as.character(text) } @@ -3439,7 +3441,7 @@ sheetVisibility <- function(wb) { return(invisible(wb)) } - for (i in 1:length(wb$worksheets)) { + for (i in seq_along(wb$worksheets)) { wb$workbook$sheets[i] <- gsub(exState0[i], value[i], wb$workbook$sheets[i], fixed = TRUE) } diff --git a/R/writeDataTable.R b/R/writeDataTable.R index e3915aa1..c8bc123d 100644 --- a/R/writeDataTable.R +++ b/R/writeDataTable.R @@ -244,7 +244,7 @@ writeDataTable <- function(wb, sheet, x, colNames[char0] <- colnames(x)[char0] <- paste0("Column", which(char0)) } } else { - colNames <- paste0("Column", 1:ncol(x)) + colNames <- paste0("Column", seq_len(ncol(x))) names(x) <- colNames } ## If zero rows, append an empty row (prevent XML from corrupting) diff --git a/src/load_workbook.cpp b/src/load_workbook.cpp index 1331b0f1..1bacc1aa 100644 --- a/src/load_workbook.cpp +++ b/src/load_workbook.cpp @@ -888,7 +888,7 @@ std::vector getChildlessNode_ss(std::string xml, std::string tag){ // [[Rcpp::export]] CharacterVector getChildlessNode(std::string xml, std::string tag) { - size_t k = tag.length(); + // size_t k = tag.length(); variable not used if(xml.length() == 0) return wrap(NA_STRING); diff --git a/tests/testthat/test-loading_workbook.R b/tests/testthat/test-loading_workbook.R index e8ec6d33..fcd971d6 100644 --- a/tests/testthat/test-loading_workbook.R +++ b/tests/testthat/test-loading_workbook.R @@ -932,4 +932,4 @@ test_that("Load and saving a file with Threaded Comments works", { # Check that wb can be saved without error expect_silent(saveWorkbook(wb, file = tempfile())) -}) \ No newline at end of file +}) diff --git a/tests/testthat/test-named_regions.R b/tests/testthat/test-named_regions.R index b8088f68..4c474dfa 100644 --- a/tests/testthat/test-named_regions.R +++ b/tests/testthat/test-named_regions.R @@ -19,8 +19,8 @@ test_that("Maintaining Named Regions on Load", { wb = wb, sheet = 1, name = "iris", - rows = 1:(nrow(iris) + 1), - cols = 1:ncol(iris) + rows = seq_len(nrow(iris) + 1), + cols = seq_len(ncol(iris)) ) @@ -369,4 +369,4 @@ test_that("Read namedRegion from specific sheet", { # Warning: Workbook has no such named region on this sheet. (Correct namedRegion, but wrong sheet selected.) expect_warning(read.xlsx(filename, sheet = "Sheet4", namedRegion = namedR, rowNames = FALSE, colNames = FALSE)) -}) \ No newline at end of file +}) diff --git a/tests/testthat/test-outlines.R b/tests/testthat/test-outlines.R index 7cca7be3..0f929b12 100644 --- a/tests/testthat/test-outlines.R +++ b/tests/testthat/test-outlines.R @@ -138,4 +138,4 @@ test_that("Consecutive calls to saveWorkbook doesn't corrupt attributes", { testthat::expect_equal(wbb$worksheets[[1]], test) unlink(c("tf", "tf2"), recursive = TRUE, force = TRUE) -}) \ No newline at end of file +}) diff --git a/tests/testthat/test-read_from_created_wb.R b/tests/testthat/test-read_from_created_wb.R index 8cf36616..b95a493b 100644 --- a/tests/testthat/test-read_from_created_wb.R +++ b/tests/testthat/test-read_from_created_wb.R @@ -322,7 +322,7 @@ test_that("Reading from new workbook cols/rows", { y <- read.xlsx(tempFile, 1, colNames = TRUE, rowNames = FALSE, rows = rows, cols = cols) df <- mtcars[sort((rows - 1)[(rows - 1) <= nrow(mtcars)]), sort(cols[cols <= ncol(mtcars)])] - rownames(df) <- 1:nrow(df) + rownames(df) <- seq_len(nrow(df)) expect_equal(object = x, expected = y) expect_equal(object = x, expected = df) @@ -366,7 +366,7 @@ test_that("Reading from new workbook cols/rows", { y <- read.xlsx(tempFile, sheet = 3, colNames = TRUE, rowNames = FALSE, rows = rows, cols = cols) df <- mtcars[sort((rows - 1)[(rows - 1) <= nrow(mtcars)]), sort(cols[cols <= ncol(mtcars)])] - rownames(df) <- 1:nrow(df) + rownames(df) <- seq_len(nrow(df)) expect_equal(object = x, expected = y, check.attributes = FALSE) expect_equal(object = df, expected = x, check.attributes = FALSE) diff --git a/tests/testthat/test-saveWorkbook.R b/tests/testthat/test-saveWorkbook.R index 071bd297..dbb5a69c 100644 --- a/tests/testthat/test-saveWorkbook.R +++ b/tests/testthat/test-saveWorkbook.R @@ -16,4 +16,4 @@ test_that("test return values for saveWorkbook", { unlink(tempFile, recursive = TRUE, force = TRUE) } -) \ No newline at end of file +) diff --git a/tests/testthat/test-write_read_equality.R b/tests/testthat/test-write_read_equality.R index 277f9204..079554dd 100644 --- a/tests/testthat/test-write_read_equality.R +++ b/tests/testthat/test-write_read_equality.R @@ -102,7 +102,7 @@ test_that("Writing then reading returns identical data.frame 2", { expect_equal(object = dateOrigin, expected = "1900-01-01", check.attributes = FALSE) - for (i in 1:ncol(x)) { + for (i in seq_len(ncol(x))) { xNoDateDetection[[i]] <- convertToDate(xNoDateDetection[[i]], origin = dateOrigin) } diff --git a/tests/testthat/test-write_xlsx_vector_args.R b/tests/testthat/test-write_xlsx_vector_args.R index ac563dcd..03793a36 100644 --- a/tests/testthat/test-write_xlsx_vector_args.R +++ b/tests/testthat/test-write_xlsx_vector_args.R @@ -110,4 +110,4 @@ test_that("write.xlsx() correctly handles colWidths", { structure(c(`1` = "1", `2` = "2", `3` = "3"), hidden = zero3) ) ) -}) \ No newline at end of file +}) diff --git a/tests/testthat/test-writing_sheet_data.R b/tests/testthat/test-writing_sheet_data.R index 2b445d04..81976e60 100644 --- a/tests/testthat/test-writing_sheet_data.R +++ b/tests/testthat/test-writing_sheet_data.R @@ -168,7 +168,7 @@ test_that("Writing sheetData rows XML - iris", { "5.935.11.87" ) - for (i in 1:length(expected_rows)) { + for (i in seq_along(expected_rows)) { expect_equal(rows[i], expected = expected_rows[i]) } @@ -226,7 +226,7 @@ test_that("Writing sheetData rows XML - mtcars", { "4321.441211094.112.7818.61142" ) - for (i in 1:length(expected_rows)) { + for (i in seq_along(expected_rows)) { expect_equal(rows[i], expected = expected_rows[i]) } })