Conversation
…s below and above the specified percentile
…contains NA or is NULL
Fix issue quantargo#7: Handling NULL values corrected
Codecov Report
@@ Coverage Diff @@
## master #2 +/- ##
=======================================
Coverage ? 100%
=======================================
Files ? 3
Lines ? 18
Branches ? 0
=======================================
Hits ? 18
Misses ? 0
Partials ? 0
Continue to review full report at Codecov.
|
* wrong implementation quantargo#1 * lacking wrong input handling quantargo#2
mannau
left a comment
There was a problem hiding this comment.
Excellent work!
Just a few minor issues regarding documentation and explicit use of any().
| @@ -1 +1,2 @@ | |||
| exportPattern("^[[:alpha:]]+") | |||
| importFrom("stats", "quantile") No newline at end of file | |||
There was a problem hiding this comment.
Have you used roxygen2 to generate the file here?
|
|
||
| transform_log <- function(x){ | ||
|
|
||
| if( sum(is.na(x)) ){ |
There was a problem hiding this comment.
Why do NA values lead to an error here?
Sidenote: Although correct it would be more explicit to use any(is.na(x))
| else if( length(x) == 0 ){ | ||
| stop("Input x is NULL") | ||
| } | ||
| else if( sum(x <= 0) ){ |
There was a problem hiding this comment.
Again, more explicit to use any(x <= 0). What could be the workaround for negative values here?
| @@ -1,10 +1,29 @@ | |||
| #' Windsorize | |||
| #' | |||
| #' Do some windsorization. | |||
There was a problem hiding this comment.
Description should be more explicit.
| q <- quantile(x, p) | ||
| x[x >= q] <- q | ||
|
|
||
| if( sum(is.na(x))){ |
| x[x >= q] <- q | ||
|
|
||
| if( sum(is.na(x))){ | ||
| stop("Input x contains value NA") |
There was a problem hiding this comment.
Do we need to throw an error here? What could be the workaround for NA values?
It's my solution