15 clean r #59
15 clean r #59
Conversation
also adds a couple of checks in case I messed up
* remove building paths from temp directory * ensure that temp files are unique * move up some on.exit() calls to make sure temp directories are removed * some small clean up
| on.exit(expr = options("OutDec" = od), add = TRUE) | ||
|
|
||
| op <- openxlsx_options() | ||
| on.exit(options(op), add = TRUE) |
There was a problem hiding this comment.
Note to self: once we have options in a working state (which hopefully will be soon), we should go through a few functions where options are included, but do not seem to provide anything usefull.
There was a problem hiding this comment.
I think a lot of them are probably not needed. The only time it seems to matter is when converting a double to a string. It's probably within the write data so we can just be careful about using using format(), (e.g., format(7 / pi, scientific = FALSE, digits = 16). I'm not entirely convinced that setting the options() is necessary.
| } | ||
|
|
||
| return(origin) | ||
| "1900-01-01" | ||
| } |
There was a problem hiding this comment.
hm, I'm not really happy with this change. makes it harder to spot what is going on and a bit weird that such a long function provides a const character. I mean it looks like it is working, but I preferred the old way
There was a problem hiding this comment.
But I'm not even sure if this is used anywhere. Remember that I wrangled with it in writeDataTable iirc.
There was a problem hiding this comment.
Oh, this does the same thing. It just doesn't bother assigning/making an origin object only to immediately return it.
Could also just simplify the last statement to:
if (grepl('date1904="1"|date1904="true"', workbook, ignore.case = TRUE)) {
"1904-01-01"
} else {
"1900-01-01"
}There was a problem hiding this comment.
Yeah I got what it does, it's just that I don't like the style. It's nothing I'd veto against, but I don't think that it is easily understandable that's all. If change is required, I think we should stick with the simplified if else one.
| xmlDir <- file.path(tempdir(), "named_regions_tmp") | ||
| xmlDir <- tempfile("named_regions_tmp") | ||
| # don't unlink on exit | ||
| on.exit(unlink(xmlDir, recursive = TRUE), add = TRUE) |
So, apparently there's this |
Codecov Report
@@ Coverage Diff @@
## main #59 +/- ##
==========================================
+ Coverage 42.65% 42.71% +0.05%
==========================================
Files 40 40
Lines 8343 8320 -23
==========================================
- Hits 3559 3554 -5
+ Misses 4784 4766 -18
Continue to review full report at Codecov.
|
|
Most likely it was there, because of the manual way Rcpp functions were integrated in |
Resolves #15 (see issue for general changes)