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

Jump to bottom

Reconsider the addition of overwrite = FALSE argument to write.xlsx #249

Closed
jeromyanglim opened this issue Aug 25, 2021 · 3 comments
Closed

Reconsider the addition of overwrite = FALSE argument to write.xlsx #249

jeromyanglim opened this issue Aug 25, 2021 · 3 comments

Comments

@jeromyanglim
Copy link

I've recently upgraded openxlsx (i.e., version 4.2.4).
The function write.xlsx now appears to have a default argument overwrite = FALSE.
This appears to be new. I don't upgrade all that often, so my apologies if it's been around for quite a while.

I think this default is undesirable for several reasons:

  • It is inconsistent with other data.frame saving functions in R. e.g., write.csv, write.table. In fact, just about any function that allows you to save a file in R allows you save that file over the top of an existing file.
  • It breaks a lot of old code that assumed that it is fine to save an excel file on top of a file.
  • It's a more common use case to be updating a file with new output than to be worried about deleting an existing file.
  • It also leads to surprising results for reproducible analyses. I.e., the first time you run your code, you get no errors, because the excel output file does not exist yet. Then you run your code a second time and you get an error (i.e., File already exists), because the file now exists. So you have to work out what is going on and then fix the error by adding the text overwrite = TRUE to the function call. This is surprising behaviour and adds to the verbosity and mental load of using the function.

A typical example of me using write.xlsx would be where I have generated some results, and I'm saving these to a folder.
If I re-run the analyses, I'll save the latest results over the top.
In general, I think you can think of R users as mature enough to know that files generated by R (e.g., using tools like write.xlsx) should not be edited in place.

So, I think the best solution would be to make the default overwrite = TRUE again especially for write.xlsx.
@JanMarvin
Copy link
Collaborator

Hi @jeromyanglim , I'm in favor of changing the default, though it leaves the potential problem, that the onus will be on the others who rely on the current behavior. What do you think @ycphs ?

@jmbarbone
Copy link
Contributor

I'd be favor of the default of FALSE. This is passed down to file.copy() where the default value for overwrite will be FALSE (unless recursive is TRUE). This was also passed intermediated to saveWorkbook() where the default value is FALSE. Since write.xlsx() is just a wrapper for creating and saving the workbook, to me it makes sense to keep the default values of the parameters consistent. I don't see it necessarily as a large burden.

However, I'm fairly certain this change was introduced unintentionally. There are no notes about it in the NEWS and the documentation for write.xlsx() is inconsistent: the argument is listed as a default of FALSE but the details indicates that the default is TRUE. For those reasons I think it's better to revert back.

@jmbarbone jmbarbone mentioned this issue Sep 4, 2021
Merged
@ycphs ycphs closed this as completed in 5c224e7 Sep 10, 2021
@tylerlittlefield
Copy link

The default of FALSE has caused quite a bit of headaches for me. Really happy to see it has been reverted on the development version! 👍🏻

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jan 4, 2025
(textproc/R-openxlsx) Updated 4.2.4 to 4.2.7.1
a91df5a 
# openxlsx 4.2.7.1

* It's now possible to insert a hyperlinked image by passing a URL,
  relative or absolute file path, or mailto string to the new
  `address` parameter of `insertImage()`.

# openxlsx 4.2.7

* Fixed warning on `dataValidation(..., type = "list")`
  ([#342](ycphs/openxlsx#342))

* Added optional argument to `loadWorkbook` to decide if empty/blank
  cells should be converted to NA_character_ (the default) or left
  blank as is

* `saveWorkbook()` now succeeds when called after the user has set
  column widths for a range of columns (e.g. 1:2), saved the workbook,
  then set column widths for a new range that is inclusive of the
  previous one (e.g. 1:5)
  ([#493](ycphs/openxlsx#493)).

## Improvements

* Improve detectDates
  ([#288](ycphs/openxlsx#288))

* Preserve window size and position, also `getWindowSize()` and
  `setWindowSize()`
  ([466](ycphs/openxlsx#466))

# openxlsx 4.2.6

* Fix external links
  ([#410](ycphs/openxlsx#410))

* Do not add unneccessary sheetPr node
  ([#409](ycphs/openxlsx#409))

* Add support for `namedRegion`s having dots and other special
  characters ([#338](ycphs/openxlsx#338)).

* Add type blanks and not blanks to conditional formatting
  ([#311](ycphs/openxlsx#311))

# openxlsx 4.2.5

## Fixes

* `openxlsx_setOp()` now works with named list
  ([#215](ycphs/openxlsx#215))

* `loadWorkbook()` imports `inlineStr`. Values remain `inlineStr` when
  writing the workbook with `saveWorkbook()`. Similar `read.xlsx` and
  `readWorkbook` import `inlineStr`.

* `read.xlsx()` no longer changes random seed
  ([#183](ycphs/openxlsx#183))

* fixed a regression that caused fonts to be read in incorrectly
  ([#207](ycphs/openxlsx#207))

* add option to save as read only recommended
  ([#201](ycphs/openxlsx#201))

* fixed writing hyperlink formulas
  ([#200](ycphs/openxlsx#200))

* `write.xlsx()` now throws an error if it doesn't have write
  permissions ([#190](ycphs/openxlsx#190))

* `write.xlsx()` now again uses the default of `overwrite = TRUE` for
  saving files ([#249](ycphs/openxlsx#249))

* `as.character.formula()` exported to warn about potential conflicts
  with other packages
  ([#312](ycphs/openxlsx#312),
  [#315](ycphs/openxlsx#315))

## Improvements

* `options()` are more consistently set in functions (see:
  [#289](ycphs/openxlsx#262))

* `Workbook$show()` no longer fails when called in a 0 sheet
  workbook([#240](ycphs/openxlsx#240))

* `read.xlsx()` again accepts `.xlsm` files
([#205](ycphs/openxlsx#205),
[#209](ycphs/openxlsx#209))

* `makeHyperlinkString()` does no longer require a sheet argument
  ([#57](ycphs/openxlsx#57),
  [#58](ycphs/openxlsx#58))

* improvements in how `openxlsx` creates temporary directories (see
  [#262](ycphs/openxlsx#262))

* `writeData()` calls `force(x)` to evaluate the object before options
  are set ([#264](ycphs/openxlsx#264))

* `createComment()` now correctly handles `integers` in `width` and
  `height` ([#275](ycphs/openxlsx#275))

* `setStyles()` accepts `halign="justify"`
  ([#305](ycphs/openxlsx#305))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jeromyanglim @JanMarvin @tylerlittlefield @jmbarbone