Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update of dataclean #43

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

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