Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove unused variable #168

Merged
merged 12 commits into from
Apr 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion R/CommentClass.R
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
28 changes: 14 additions & 14 deletions R/WorkbookClass.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not seq_len(nSheets)?

Copy link
Owner Author

@ycphs ycphs Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is the equivalent and nSheets is just used for this loop.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, yes. sleepy me 👍

## Write drawing i (will always exist) skip those that are empty
if (any(drawings[[i]] != "")) {
write_file(
Expand Down Expand Up @@ -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]],
Expand All @@ -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
)
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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]]

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]]
}

Expand Down
6 changes: 3 additions & 3 deletions R/loadWorkbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion R/openxlsxCoerce.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion R/workbook_column_widths.R
Original file line number Diff line number Diff line change
Expand Up @@ -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]])
Expand Down
2 changes: 1 addition & 1 deletion R/workbook_read_workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions R/workbook_write_data.R
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
}
}
Expand Down Expand Up @@ -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)
})

Expand Down
4 changes: 2 additions & 2 deletions R/worksheet_class.R
Original file line number Diff line number Diff line change
Expand Up @@ -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("<hyperlinks>", paste(sapply(1:length(h_inds), function(i) hyperlinks[[i]]$to_xml(h_inds[i])), collapse = ""), "</hyperlinks>"), collapse = "")
h_inds <- paste0(seq_along(hyperlinks), "h")
xml <- paste(xml, paste("<hyperlinks>", paste(sapply(seq_along(h_inds), function(i) hyperlinks[[i]]$to_xml(h_inds[i])), collapse = ""), "</hyperlinks>"), collapse = "")
}


Expand Down
18 changes: 10 additions & 8 deletions R/wrappers.R
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -863,15 +864,15 @@ 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)))
}

## 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)))
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)
}


Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion R/writeDataTable.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/load_workbook.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ std::vector<std::string> 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);

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-loading_workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -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()))

})
})
6 changes: 3 additions & 3 deletions tests/testthat/test-named_regions.R
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)


Expand Down Expand Up @@ -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))
})
})
2 changes: 1 addition & 1 deletion tests/testthat/test-outlines.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
4 changes: 2 additions & 2 deletions tests/testthat/test-read_from_created_wb.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-saveWorkbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ test_that("test return values for saveWorkbook", {
unlink(tempFile, recursive = TRUE, force = TRUE)

}
)
)
2 changes: 1 addition & 1 deletion tests/testthat/test-write_read_equality.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-write_xlsx_vector_args.R
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,4 @@ test_that("write.xlsx() correctly handles colWidths", {
structure(c(`1` = "1", `2` = "2", `3` = "3"), hidden = zero3)
)
)
})
})
4 changes: 2 additions & 2 deletions tests/testthat/test-writing_sheet_data.R
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ test_that("Writing sheetData rows XML - iris", {
"<row r=\"151\"><c r=\"A151\" t=\"n\"><v>5.9</v></c><c r=\"B151\" t=\"n\"><v>3</v></c><c r=\"C151\" t=\"n\"><v>5.1</v></c><c r=\"D151\" t=\"n\"><v>1.8</v></c><c r=\"E151\" t=\"s\"><v>7</v></c></row>"
)

for (i in 1:length(expected_rows)) {
for (i in seq_along(expected_rows)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally would have replaced it with seq_len(length(...)). yours does look better, but I've been bitten enough not to blindly trust seq_along for anything not data.frame related.

expect_equal(rows[i], expected = expected_rows[i])
}

Expand Down Expand Up @@ -226,7 +226,7 @@ test_that("Writing sheetData rows XML - mtcars", {
"<row r=\"33\"><c r=\"A33\" t=\"s\"><v>43</v></c><c r=\"B33\" t=\"n\"><v>21.4</v></c><c r=\"C33\" t=\"n\"><v>4</v></c><c r=\"D33\" t=\"n\"><v>121</v></c><c r=\"E33\" t=\"n\"><v>109</v></c><c r=\"F33\" t=\"n\"><v>4.11</v></c><c r=\"G33\" t=\"n\"><v>2.78</v></c><c r=\"H33\" t=\"n\"><v>18.6</v></c><c r=\"I33\" t=\"n\"><v>1</v></c><c r=\"J33\" t=\"n\"><v>1</v></c><c r=\"K33\" t=\"n\"><v>4</v></c><c r=\"L33\" t=\"n\"><v>2</v></c></row>"
)

for (i in 1:length(expected_rows)) {
for (i in seq_along(expected_rows)) {
expect_equal(rows[i], expected = expected_rows[i])
}
})