-
Notifications
You must be signed in to change notification settings - Fork 14
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
[ideas][v2] thinking about API changes and deprecation warnings #1213
Comments
I could open this as a separate issue but I’d love to see the package add formal support for lists of workbooks that could be manipulated using map or lapply and eventually output as a series of Excel files. The gt_group function is a possible model for structure: https://gt.rstudio.com/reference/gt_group.html I definitely have more ideas for API tweaks and new features but will get those thoughts organized before sharing more. |
Hello @elipousson , this is a somewhat unusual request as I am currently aware of three main complaints from users:
And while (1) and (2) are rather general complaints, (3) - from a few thousand lines on - is a legitimate complaint. And your desire to create lists of workbooks somehow increases memory consumption :) Back to topic, you want something like this? library(openxlsx2)
## create two workbooks
wb1 <- write_xlsx(x = iris)
wb2 <- write_xlsx(x = cars)
## create a list
wb_list <- list(wb1, wb2)
## apply color to head
head_in_color <- function(x, color) {
openxlsx2:::assert_workbook(x)
openxlsx2:::assert_class(color, "wbColour")
## make a memory copy
x <- x$clone(deep = TRUE)
## get dims and apply color
dims <- wb_dims(x = head(wb_to_df(x)))
x$add_fill(dims = dims, color = color)
}
## lapply
wb_list_2 <- lapply(wb_list, head_in_color, color = wb_color("yellow"))
if (interactive()) {
wb_list_2[[1]]$open()
wb_list_2[[2]]$open()
}
## purrr::map
wb_list_3 <- purrr::map(wb_list, head_in_color, color = wb_color("orange"))
if (interactive()) {
wb_list_3[[1]]$open()
wb_list_3[[2]]$open()
}
## original list untouched
if (interactive()) {
wb_list[[1]]$open()
wb_list[[2]]$open()
} |
This issue is to bundle some ideas for a possible v2 version.
So far
openxlsx2
avoids breaking changes if possible and there is no need to change anything yet. That's why this issue is not a, it will be implemented soon, but a could come and may take another six months or longer.openxlsx2.soon_deprecated
We still allow some deprecated usages, e.g. entering
rows
/cols
in various functions. It is probably time that we enable theopenxlsx2.soon_deprecated
warning. We should deliver this warning for some time, maybe six months, and then remove the old behavior (and the warning).standardize()
Although the addition of
standardize(...)
worked well, there is a fee to pay. We do not strictly check the possible arguments passed to each function. Therefore, passing arguments to functions that are not used will not cause a warning. Therefore, fixing this problem should be at least a medium-term goal.write data cleanup
In addition, the functions
wb_add_data()
/wb_add_formula()
/wb_add_data_table()
could probably be tightened up a little. Currently, the process looks something like this:It is because we have been exporting the original
do_write_data()
function for a long time. And all functions perform various checks, data changes and shaping at some point. We check for hyperlinks at various stages, and untilwrite_data2()
, non-data frame input is allowed. This should be cleaned up as it may break the currently documented behavior.Arguments cleanup
For the recently added arguments, we have avoided changes in the order of the arguments. The arguments were not added in the best sensible order, but at the end of the previous arguments. Some arguments should allow different inputs or no inputs at all. See e.g. #1153
Argument checking
We allow various unchecked inputs for many arguments. Recently I was confused about
wb_set_sheetview(view = ...)
. I used an unexpected input and was not alerted with a warning. It's probably time we introducematch.args()
for all inputs (perhaps with an optional override option) and clean up the remaining camel case along the way.Input, ideas and suggestions are welcome
I am not the only one using this package, and possibly many are affected by the changes to the API. So any input on planned or desired changes is welcome, but please be reasonable. It is unlikely that
openxlsx2
will be rewritten on a large scale.The text was updated successfully, but these errors were encountered: