Conversation
issue quantargo#31- detailed description of a function added
mannau
left a comment
mannau
left a comment
There was a problem hiding this comment.
Thanks for your pull request! I've attached a few comments.
| transform_log<-function(x){ | ||
| if(!is.numeric(x))stop("function is expecting only numeric values") | ||
| x_nan<-is.na(x) | ||
| x[x_nan]<-1 |
There was a problem hiding this comment.
NA values are subsequently converted to ones and later to zeros through log(1). Is this behaviour intentional?
| @@ -1,4 +1,6 @@ | |||
| #' Meanimputation | |||
| #' Calculates mean of a given vector, ignores NA values. | |||
There was a problem hiding this comment.
This function actually replaces all NA values with the mean of a given vector.
| if(!is.numeric(x))stop("function is expecting only numeric values") | ||
| x_nan<-is.na(x) | ||
| x[x_nan]<-1 | ||
| ifelse(x<0,"OK", warning("input vector contains negative values, turned into NA")) |
There was a problem hiding this comment.
This statement gives a warning for EVERY negative value in x.
It would also be sufficient to do a
if(any(x < 0)) {
warning("input vector contains negative values, turned into NA")
}
| y<-log(x[x>=0]) | ||
| x[x>=0]<-y | ||
| x[x<0]<-NA | ||
| x[x_nan]<-NA |
There was a problem hiding this comment.
And here the zeros are converted again to NAs (see previous comment).
This part seems could be simplified.
| x[x >= q] <- q | ||
| if(is.null(x))stop("vector is empty") | ||
| y<-x[!is.na(x)] | ||
| if(length(y)==0)stop("vector contains only NAs") |
There was a problem hiding this comment.
The condition could be expressed more clearly using
if( all(is.na(x)) ) stop("vector contains only NAs")
Codecov Report
@@ Coverage Diff @@
## master #43 +/- ##
========================================
Coverage ? 0.00%
========================================
Files ? 3
Lines ? 19
Branches ? 0
========================================
Hits ? 0
Misses ? 19
Partials ? 0 Continue to review full report at Codecov.
|
3 participants
I have only one more error, and it concerns PDF version of manual,
and I don't know why