Improvement of datacleaner package #24
Conversation
…ent of NA and NULL
Codecov Report
@@ Coverage Diff @@
## master #24 +/- ##
========================================
Coverage ? 91.3%
========================================
Files ? 3
Lines ? 23
Branches ? 0
========================================
Hits ? 21
Misses ? 2
Partials ? 0
Continue to review full report at Codecov.
|
mannau
left a comment
mannau
left a comment
There was a problem hiding this comment.
Great work!
Just a few minor comments/issues
| meanimpute <- function(x) { | ||
|
|
||
| if(is.null(x)) {stop("Input vector cannot be NULL.")} | ||
| if(all(is.na(x))) {stop("Input vector should contain at least one numeric element.")} |
There was a problem hiding this comment.
Being a bit picky here: ´Input vector should contain at least one numeric element which is not NA´.
| @@ -0,0 +1,20 @@ | |||
| #' transform_log | |||
| #' | |||
| #' log-transformation of a numeric vector. For details about log-transformation please see you basic school math textbook. | |||
There was a problem hiding this comment.
Please remove the last sentence and either think of
- Why log transformations are typically used and/or
- Hint to a Web link which describes the transformation in detail
| transform_log <- function(x){ | ||
|
|
||
| if( is.null(x) ) stop("Input vector is not allowed to be NULL.") | ||
| if( any(is.na(x)) ) stop("There is at least one NA value in input vector.") |
There was a problem hiding this comment.
The stop() could also be turned into a warning()
| if( is.null(x) ) stop("Input vector is not allowed to be NULL.") | ||
| if( any(is.na(x)) ) stop("There is at least one NA value in input vector.") | ||
| if( any(x <= 0) ) stop("There is at least one negative value.") | ||
| if( any(is.numeric(x) == FALSE) ) stop("There is at least one non-numeric value.") |
There was a problem hiding this comment.
Better here use the negation sign: !is.numeric(x)
|
|
||
| if(is.null(x)) {stop("Input vector cannot be NULL.")} | ||
| if(any(is.na(x))) {stop("There should be no NA's in input vector.")} | ||
| if(all(is.numeric(x)==FALSE)) {stop("There should only numeric values in the input vector.")} |
| x | ||
|
|
||
| if(is.null(x)) {stop("Input vector cannot be NULL.")} | ||
| if(any(is.na(x))) {stop("There should be no NA's in input vector.")} |
There was a problem hiding this comment.
- Why are no
NA's allowed? What could be the workaround here to still windsorize and keepNAs? - Typo: There should be no NA's in the input...
| if(any(is.na(x))) {stop("There should be no NA's in input vector.")} | ||
| if(all(is.numeric(x)==FALSE)) {stop("There should only numeric values in the input vector.")} | ||
|
|
||
| if(is.na(p)==TRUE) {stop("Input quantile should be a number between 0 and 1 ")} |
There was a problem hiding this comment.
The test does not match with the error message.
| if(all(is.numeric(x)==FALSE)) {stop("There should only numeric values in the input vector.")} | ||
|
|
||
| if(is.na(p)==TRUE) {stop("Input quantile should be a number between 0 and 1 ")} | ||
| if(is.numeric(p)==FALSE) {stop("Input quantile should be a number between 0 and 1 ")} |
3 participants
Following updates should significantly improve the package.