read_vc(), write_vc(), rm_data(): default root #6
Conversation
There was a problem hiding this comment.
Good suggestion. Can you make the minor changes and update the documentation? Use 'Install and restart' in RStudio and then 'Check package'. If the checks fail on documentation issue, then tick the 'Generate documentation with Roxygen box' in the 'Build configuration'.
The package is currently under review, so I have to keep the master stable for a while.
Thanks @ThierryO for the code suggestion Co-Authored-By: florisvdh <[email protected]>
|
Some changes have been committed, and have also been applied to the |
Codecov Report
@@ Coverage Diff @@
## develop #6 +/- ##
======================================
Coverage 100% 100%
======================================
Files 7 7
Lines 340 355 +15
======================================
+ Hits 340 355 +15
Continue to review full report at Codecov.
|
florisvdh
changed the title
R/write_vc.R
Outdated
| root = ".", | ||
| sorting = sorting, | ||
| override = FALSE, | ||
| optimize = TRUE, |
There was a problem hiding this comment.
@ThierryO There is a problem here, how to fix this? While the intention is to set defaults for override and optimize, i.e. the part that says
definition = function(
x, file, root, sorting, override = FALSE, optimize = TRUE, ...)
, the if (missing(root)) part, if evaluated as TRUE, returns the write_vc function with override and optimize set to fixed values: these values now overrule the values originally set by the user.
So, when doing something like mydataframe %>% write_vc(file = "myfilename", sorting = mysortingvector, override = TRUE), while the file already existed and the sorting of the file has changed, I nevertheless get following error:
Error: old data has different variable types or sorting, use override = TRUE
from the traceback in RStudio:
14.
stop(call. = FALSE, "old data has different variable types or sorting, use override = TRUE") at write_vc.R#213
13.
compare_meta(metadata, old_metadata) at write_vc.R#146
12.
write_vc(x = x, file = file, root = ".", sorting = sorting, override = FALSE, optimize = TRUE, ...) at write_vc.R#30
11.
write_vc(x = x, file = file, root = ".", sorting = sorting, override = FALSE, optimize = TRUE, ...) at write_vc.R#43
10.
write_vc(., file = "myfilename", sorting = mysortingvector, override = TRUE) at write_vc.R#30
9.
write_vc(., file = "myfilename", sorting = mysortingvector, override = TRUE)
There was a problem hiding this comment.
The same problem is probably present in the rm_data() function
There was a problem hiding this comment.
when you call a function recursively, you need to pass the arguments rather than hard coding defaults.
R/write_vc.R
Outdated
| file = file, | ||
| root = ".", | ||
| sorting = sorting, | ||
| override = FALSE, |
There was a problem hiding this comment.
Use override = override and optimize = optimize
R/rm_data.R
Outdated
| ){ | ||
| if (missing(root)) { | ||
| return(rm_data(root = ".", | ||
| path = NULL, |
There was a problem hiding this comment.
use path = path, type = type and recursive = recursive
Borrowed code from
write_vc()in order to makeread_vc()throw an equally understandable error when the 'root' argument is not specified. Review carefully though; I'm not familiar with error handling and such.BTW, why not setting a default for the root argument, like
"."?