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.

Sign up for GitHub

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

Report failure when write.xlsx fails. #208

Merged
merged 19 commits into from
Aug 2, 2021
+205 −104
Merged

Report failure when write.xlsx fails. #208

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
930c79f
version bumps
jeff-m-sullivan Jun 25, 2021
ef83417
package names added to world list
jeff-m-sullivan Jun 25, 2021
cea0aad
pre-commit tidied and fs package added
jeff-m-sullivan Jun 25, 2021
7e6a2b4
test to fail when write permissions denied doesn't cause error
jeff-m-sullivan Jun 25, 2021
3d8931e
Revert "test to fail when write permissions denied doesn't cause error"
jeff-m-sullivan Jun 25, 2021
b6d893d
test whether write.xlsx throws an error without proper permissions
jeff-m-sullivan Jun 25, 2021
ea8389f
file.copy throws warnings, not errors
jeff-m-sullivan Jun 25, 2021
045ef4e
apply styler before substantive change
jeff-m-sullivan Jun 25, 2021
ec4505a
removing tryCatch, which was suppressing helpful errors
jeff-m-sullivan Jun 25, 2021
3f59b9c
clean up whitespace
jeff-m-sullivan Jun 25, 2021
a8560fd
revdepcheck run - No changes to commit
actions-user Jun 25, 2021
6d68df9
Merge branch 'master' into error-failed-write
jeff-m-sullivan Jul 3, 2021
3aff705
revdepcheck run - No changes to commit
actions-user Jul 3, 2021
4fe562d
Merge branch 'master' into error-failed-write
jeff-m-sullivan Jul 6, 2021
d737228
revdepcheck run - No changes to commit
actions-user Jul 6, 2021
0bc49fb
Merge branch 'master' into error-failed-write
jeff-m-sullivan Jul 8, 2021
a03fc29
Merge branch 'master' into error-failed-write
jeff-m-sullivan Jul 19, 2021
ed1f0d8
test for specific permission denied warning
jeff-m-sullivan Jul 19, 2021
ec229fe
remove dependency on fs package
jeff-m-sullivan Jul 23, 2021
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
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# R specific hooks: https://github.com/lorenzwalthert/precommit
repos:
- repo: https://github.com/lorenzwalthert/precommit
rev: v0.1.2
rev: v0.1.3.9014
hooks:
- id: style-files
args: [--style_pkg=styler, --style_fun=tidyverse_style]
Expand Down Expand Up @@ -38,7 +38,7 @@ repos:
- id: no-browser-statement
- id: deps-in-desc
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v1.2.3
rev: v4.0.1
hooks:
- id: check-added-large-files
args: ['--maxkb=200']
Expand Down
27 changes: 15 additions & 12 deletions DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ Package: openxlsx
Title: Read, Write and Edit xlsx Files
Version: 4.2.4.9000
Date: 2021-06-08
Language: en-US
Authors@R:
c(person(given = "Philipp",
family = "Schauberger",
Expand All @@ -21,18 +20,18 @@ Authors@R:
role = "ctb"),
person(given = "Jan Marvin",
family = "Garbuszus",
email = "[email protected]",
role = "ctb"),
role = "ctb",
email = "[email protected]"),
person(given = "Jordan Mark",
family = "Barbone",
role = "ctb",
email = "[email protected]",
comment = c(ORCID = "0000-0001-9788-3628")))
Description: Simplifies the creation of Excel .xlsx files by
providing a high level interface to writing, styling and editing
worksheets. Through the use of 'Rcpp', read/write times are comparable
to the 'xlsx' and 'XLConnect' packages with the added benefit of
removing the dependency on Java.
Description: Simplifies the creation of Excel .xlsx files by providing a
high level interface to writing, styling and editing worksheets.
Through the use of 'Rcpp', read/write times are comparable to the
'xlsx' and 'XLConnect' packages with the added benefit of removing the
dependency on Java.
License: MIT + file LICENSE
URL: https://ycphs.github.io/openxlsx/index.html,
https://github.com/ycphs/openxlsx
Expand All @@ -44,19 +43,23 @@ Imports:
methods,
Rcpp,
stats,
stringi,
utils,
zip,
stringi
zip
Suggests:
fs (>= 1.5.0),
knitr,
testthat,
rmarkdown,
roxygen2,
rmarkdown
testthat
LinkingTo:
Rcpp
VignetteBuilder:
knitr
Remotes:
r-lib/fs
Encoding: UTF-8
Language: en-US
RoxygenNote: 7.1.1
Collate:
'CommentClass.R'
Expand Down
171 changes: 81 additions & 90 deletions R/wrappers.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
#' category = "something"
#' )
createWorkbook <- function(creator = ifelse(.Platform$OS.type == "windows", Sys.getenv("USERNAME"), Sys.getenv("USER")),
title = NULL,
subject = NULL,
category = NULL) {
title = NULL,
subject = NULL,
category = NULL) {
od <- getOption("OutDec")
options("OutDec" = ".")
on.exit(expr = options("OutDec" = od), add = TRUE)
Expand Down Expand Up @@ -119,20 +119,15 @@ saveWorkbook <- function(wb, file, overwrite = FALSE, returnValue = FALSE) {

xlsx_file <- wb$saveWorkbook()

result<-tryCatch(file.copy(from = xlsx_file, to = file, overwrite = overwrite),
error = function(e) e, warning = function(w) w)



result <- file.copy(from = xlsx_file, to = file, overwrite = overwrite)

## delete temporary dir
unlink(dirname(xlsx_file), force = TRUE, recursive = TRUE)
if(returnValue == FALSE){
if (returnValue == FALSE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ycphs
Do you know if there any reason why we wouldn't want or be able to simply return the result of the file.copy()? Does using invisible(1) satisfy anything?

Copy link
Owner

Choose a reason for hiding this comment

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

Initially there was no return value and the feature request to have an optional made it necessary to have a default with no return value.

Copy link
Contributor

Choose a reason for hiding this comment

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

The default value returned was invisible(1) which I don't see as useful as having the return value be the result of file.copy() instead -- especially since if file.copy() fails saveWorkbook() essentially fails. So I guess I'm asking: should invisible(1) ever be returned or should the result of just be file.copy() (invisibly) returned?

Copy link
Owner

Choose a reason for hiding this comment

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

yes that would make more sense and it helps to actually and leads to the initial error.

invisible(1)
}else{
} else {
return(result)
}

}


Expand Down Expand Up @@ -362,24 +357,22 @@ sheets <- function(wb) {
#' \dontrun{
#' saveWorkbook(wb, "addWorksheetExample.xlsx", overwrite = TRUE)
#' }
addWorksheet <- function(
wb,
sheetName,
gridLines = openxlsx_getOp("gridLines", TRUE),
tabColour = NULL,
zoom = 100,
header = openxlsx_getOp("header"),
footer = openxlsx_getOp("footer"),
evenHeader = openxlsx_getOp("evenHeader"),
evenFooter = openxlsx_getOp("evenFooter"),
firstHeader = openxlsx_getOp("firstHeader"),
firstFooter = openxlsx_getOp("firstFooter"),
visible = TRUE,
paperSize = openxlsx_getOp("paperSize", 9),
orientation = openxlsx_getOp("orientation", "portrait"),
vdpi = openxlsx_getOp("vdpi", 300),
hdpi = openxlsx_getOp("hdpi", 300)
) {
addWorksheet <- function(wb,
sheetName,
gridLines = openxlsx_getOp("gridLines", TRUE),
tabColour = NULL,
zoom = 100,
header = openxlsx_getOp("header"),
footer = openxlsx_getOp("footer"),
evenHeader = openxlsx_getOp("evenHeader"),
evenFooter = openxlsx_getOp("evenFooter"),
firstHeader = openxlsx_getOp("firstHeader"),
firstFooter = openxlsx_getOp("firstFooter"),
visible = TRUE,
paperSize = openxlsx_getOp("paperSize", 9),
orientation = openxlsx_getOp("orientation", "portrait"),
vdpi = openxlsx_getOp("vdpi", 300),
hdpi = openxlsx_getOp("hdpi", 300)) {
od <- getOption("OutDec")
options("OutDec" = ".")
on.exit(expr = options("OutDec" = od), add = TRUE)
Expand All @@ -393,11 +386,11 @@ addWorksheet <- function(
}

# Set NULL defaults
gridLines <- gridLines %||% TRUE
paperSize <- paperSize %||% 9
gridLines <- gridLines %||% TRUE
paperSize <- paperSize %||% 9
orientation <- orientation %||% "portrait"
vdpi <- vdpi %||% 300
hdpi <- hdpi %||% 300
vdpi <- vdpi %||% 300
hdpi <- hdpi %||% 300

if (tolower(sheetName) %in% tolower(wb$sheet_names)) {
stop(paste0("A worksheet by the name '", sheetName, "' already exists! Sheet names must be unique case-insensitive."))
Expand Down Expand Up @@ -608,8 +601,10 @@ 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(seq_along(d), function(i) sum(d[[i]] * (26^(
seq_along(d[[i]]) - 1)))))
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 @@ -748,25 +743,23 @@ convertFromExcelRef <- function(col) {
#'
#' # supply all colours
#' createStyle(border = "TopBottomLeft", borderColour = c("red", "yellow", "green"))
createStyle <- function(
fontName = NULL,
fontSize = NULL,
fontColour = NULL,
numFmt = openxlsx_getOp("numFmt", "GENERAL"),
border = NULL,
borderColour = openxlsx_getOp("borderColour", "black"),
borderStyle = openxlsx_getOp("borderStyle", "thin"),
bgFill = NULL,
fgFill = NULL,
halign = NULL,
valign = NULL,
textDecoration = NULL,
wrapText = FALSE,
textRotation = NULL,
indent = NULL,
locked = NULL,
hidden = NULL
) {
createStyle <- function(fontName = NULL,
fontSize = NULL,
fontColour = NULL,
numFmt = openxlsx_getOp("numFmt", "GENERAL"),
border = NULL,
borderColour = openxlsx_getOp("borderColour", "black"),
borderStyle = openxlsx_getOp("borderStyle", "thin"),
bgFill = NULL,
fgFill = NULL,
halign = NULL,
valign = NULL,
textDecoration = NULL,
wrapText = FALSE,
textRotation = NULL,
indent = NULL,
locked = NULL,
hidden = NULL) {

### Error checking
od <- getOption("OutDec")
Expand Down Expand Up @@ -1030,15 +1023,13 @@ createStyle <- function(
#' \dontrun{
#' saveWorkbook(wb, "addStyleExample.xlsx", overwrite = TRUE)
#' }
addStyle <- function(
wb,
sheet,
style,
rows,
cols,
gridExpand = FALSE,
stack = FALSE
) {
addStyle <- function(wb,
sheet,
style,
rows,
cols,
gridExpand = FALSE,
stack = FALSE) {
od <- getOption("OutDec")
options("OutDec" = ".")
on.exit(expr = options("OutDec" = od), add = TRUE)
Expand Down Expand Up @@ -1113,9 +1104,9 @@ getCellRefs <- function(cellCoords) {


if (!("numeric" %in% sapply(cellCoords[, 1], class) |
"integer" %in% sapply(cellCoords[, 1], class))
& ("numeric" %in% sapply(cellCoords[, 2], class) |
"integer" %in% sapply(cellCoords[, 2], class))
"integer" %in% sapply(cellCoords[, 1], class)) &
("numeric" %in% sapply(cellCoords[, 2], class) |
"integer" %in% sapply(cellCoords[, 2], class))

) {
stop("Provide a data.frame containing integers!")
Expand Down Expand Up @@ -1434,7 +1425,9 @@ setColWidths <- function(wb, sheet, cols, widths = 8.43, hidden = rep(FALSE, len
}

# should do nothing if the cols' length is zero
if (length(cols) == 0L) return(invisible(0))
if (length(cols) == 0L) {
return(invisible(0))
}

if (length(widths) > length(cols)) {
stop("More widths than columns supplied.")
Expand Down Expand Up @@ -1640,7 +1633,7 @@ removeRowHeights <- function(wb, sheet, rows) {
#' saveWorkbook(wb, "insertPlotExample.xlsx", overwrite = TRUE)
#' }
insertPlot <- function(wb, sheet, width = 6, height = 4, xy = NULL,
startRow = 1, startCol = 1, fileType = "png", units = "in", dpi = 300) {
startRow = 1, startCol = 1, fileType = "png", units = "in", dpi = 300) {
od <- getOption("OutDec")
options("OutDec" = ".")
on.exit(expr = options("OutDec" = od), add = TRUE)
Expand Down Expand Up @@ -1972,12 +1965,12 @@ getBaseFont <- function(wb) {
#' saveWorkbook(wb, "setHeaderFooterExample.xlsx", overwrite = TRUE)
#' }
setHeaderFooter <- function(wb, sheet,
header = NULL,
footer = NULL,
evenHeader = NULL,
evenFooter = NULL,
firstHeader = NULL,
firstFooter = NULL) {
header = NULL,
footer = NULL,
evenHeader = NULL,
evenFooter = NULL,
firstHeader = NULL,
firstFooter = NULL) {
if (!"Workbook" %in% class(wb)) {
stop("First argument must be a Workbook.")
}
Expand Down Expand Up @@ -2167,11 +2160,11 @@ setHeaderFooter <- function(wb, sheet,
#' saveWorkbook(wb, "pageSetupExample.xlsx", overwrite = TRUE)
#' }
pageSetup <- function(wb, sheet, orientation = NULL, scale = 100,
left = 0.7, right = 0.7, top = 0.75, bottom = 0.75,
header = 0.3, footer = 0.3,
fitToWidth = FALSE, fitToHeight = FALSE, paperSize = NULL,
printTitleRows = NULL, printTitleCols = NULL,
summaryRow = NULL, summaryCol = NULL) {
left = 0.7, right = 0.7, top = 0.75, bottom = 0.75,
header = 0.3, footer = 0.3,
fitToWidth = FALSE, fitToHeight = FALSE, paperSize = NULL,
printTitleRows = NULL, printTitleCols = NULL,
summaryRow = NULL, summaryCol = NULL) {
od <- getOption("OutDec")
options("OutDec" = ".")
on.exit(expr = options("OutDec" = od), add = TRUE)
Expand Down Expand Up @@ -2236,7 +2229,6 @@ pageSetup <- function(wb, sheet, orientation = NULL, scale = 100,
outlinepr <- ""

if (!is.null(summaryRow)) {

if (!validRow(summaryRow)) {
stop("Invalid \`summaryRow\` option. Must be one of \"Above\" or \"Below\".")
} else if (tolower(summaryRow) == "above") {
Expand All @@ -2247,7 +2239,6 @@ pageSetup <- function(wb, sheet, orientation = NULL, scale = 100,
}

if (!is.null(summaryCol)) {

if (!validCol(summaryCol)) {
stop("Invalid \`summaryCol\` option. Must be one of \"Left\" or \"Right\".")
} else if (tolower(summaryCol) == "left") {
Expand Down Expand Up @@ -2353,12 +2344,12 @@ pageSetup <- function(wb, sheet, orientation = NULL, scale = 100,
#' saveWorkbook(wb, "pageSetupExample.xlsx", overwrite = TRUE)
#' }
protectWorksheet <- function(wb, sheet, protect = TRUE, password = NULL,
lockSelectingLockedCells = NULL, lockSelectingUnlockedCells = NULL,
lockFormattingCells = NULL, lockFormattingColumns = NULL, lockFormattingRows = NULL,
lockInsertingColumns = NULL, lockInsertingRows = NULL, lockInsertingHyperlinks = NULL,
lockDeletingColumns = NULL, lockDeletingRows = NULL,
lockSorting = NULL, lockAutoFilter = NULL, lockPivotTables = NULL,
lockObjects = NULL, lockScenarios = NULL) {
lockSelectingLockedCells = NULL, lockSelectingUnlockedCells = NULL,
lockFormattingCells = NULL, lockFormattingColumns = NULL, lockFormattingRows = NULL,
lockInsertingColumns = NULL, lockInsertingRows = NULL, lockInsertingHyperlinks = NULL,
lockDeletingColumns = NULL, lockDeletingRows = NULL,
lockSorting = NULL, lockAutoFilter = NULL, lockPivotTables = NULL,
lockObjects = NULL, lockScenarios = NULL) {
if (!"Workbook" %in% class(wb)) {
stop("First argument must be a Workbook.")
}
Expand Down Expand Up @@ -2572,12 +2563,12 @@ worksheetOrder <- function(wb) {
stop("Elements of order are greater than the number of worksheets")
}


old_ActiveSheet <- wb$ActiveSheet

wb$sheetOrder <- value
wb$setactiveSheet(old_ActiveSheet)


invisible(wb)
}
Expand Down
Loading