ycphs / openxlsx

openxlsx - a fast way to read and write complex xslx files
https://ycphs.github.io/openxlsx/
Other
220 stars 74 forks source link

Bugfix/fix vignette examples #382

Closed jmuhlenkamp closed 1 year ago

jmuhlenkamp commented 1 year ago

This PR fixes examples in vignettes here https://ycphs.github.io/openxlsx/articles/Introduction.html that fail to run when copied and pasted with openxlsx version 4.2.5

Specifically, in the existing examples

A <- readJPEG(file.path(path.package("openxlsx"), "einstein.jpg")) results in file not found.

## remove example files for cran test
if (identical(Sys.getenv("NOT_CRAN", unset = "true"), "false")) {
file_list<-list.files(pattern="\\.xlsx",recursive = T)
file_list<-fl[!grepl("inst/extdata",file_list)&!grepl("man/",file_list)]

if(length(file_list)>0){
rm(file_list)
}

results in unclosed braces that may be difficult for some users to debug. Note: this appears already fixed in the .Rmd but not in the published .html?

df <- read.xlsx(system.file("readTest.xlsx", package = "openxlsx"), sheet = 4) results in file not found.

I hope this helps, please let me know if there are other steps I need to do to make this merge ready. Thanks! Jason

jmuhlenkamp commented 1 year ago

Please note: I did NOT update any .html files as these are included in .gitignore. Let me know if any and which ones need updated (or add necessary commits yourself :-), thanks!

JanMarvin commented 1 year ago

Hi @jmuhlenkamp , iirc this is solved in development. Shame on us that we do not mention this branch publicly. [edit] Oh it's more than the missing }. Thanks I'll look more closely this week. Thanks for bringing this up.

codecov-commenter commented 1 year ago

Codecov Report

Merging #382 (9e0ccbe) into master (33c6203) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #382   +/-   ##
=======================================
  Coverage   67.76%   67.76%           
=======================================
  Files          34       34           
  Lines        8931     8931           
=======================================
  Hits         6052     6052           
  Misses       2879     2879           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jmuhlenkamp commented 1 year ago

Ok, no worries. If this PR is/was helpful in any way, great! If not, no big deal. Thanks for the useful package!

JanMarvin commented 1 year ago

@jmuhlenkamp Thanks again for the suggest. I've used your initial approach to do minor cleanups in the vignettes. I've switched all evals to TRUE and made sure that it ran without errors/warnings. It's now a bit messier than your original PR, but that's because the vignettes themselves are a bit messy.