Skip to content

Conversation

@MarkoBarzic
Copy link

I have only one more error, and it concerns PDF version of manual,
and I don't know why

Copy link
Member

@mannau mannau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition could be expressed more clearly using

if( all(is.na(x)) ) stop("vector contains only NAs")

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@c4b35b1). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4b35b1...3706cdc. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants