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

Fix bug in zv & corr preProcess #966

Merged
merged 2 commits into from Nov 15, 2018
Merged

Fix bug in zv & corr preProcess #966

merged 2 commits into from Nov 15, 2018

Conversation

@floriandeboissieu
Copy link
Contributor

@floriandeboissieu floriandeboissieu commented Nov 14, 2018

In preProcess with methods "zv" and "corr", if there is zero variance variable to remove, it must be taken into account for correlation computation, otherwise correlation fails to compute.

In preProcess with methods "zv" and "corr", if there is zero variance variable to remove, it must be taken into account for correlation computation, otherwise correlation fails to compute.
nothing more, just more compact syntax
@codecov-io
Copy link

@codecov-io codecov-io commented Nov 14, 2018

Codecov Report

Merging #966 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #966   +/-   ##
=======================================
  Coverage   19.54%   19.54%           
=======================================
  Files          90       90           
  Lines       12075    12075           
=======================================
  Hits         2360     2360           
  Misses       9715     9715

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 47dd5ff...317f93e. Read the comment docs.

@topepo
Copy link
Owner

@topepo topepo commented Nov 15, 2018

Thanks.

I'm going to add the additional step of adding the "zv" method when anyone chooses "corr" in the method argument.

@topepo topepo merged commit 37c6688 into topepo:master Nov 15, 2018
3 checks passed
3 checks passed
codecov/patch Coverage not affected when comparing 47dd5ff...317f93e
Details
codecov/project 19.54% remains the same compared to 47dd5ff
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
topepo added a commit that referenced this pull request Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.