zachmayer / caretEnsemble

caret models all the way down :turtle:
http://zachmayer.github.io/caretEnsemble/
Other
226 stars 75 forks source link

modified extractBestPreds to handle NAs present in adaptive_cv #95

Closed rlesca01 closed 9 years ago

rlesca01 commented 9 years ago

This fixes #94. Tested it with boot, cv, and adaptive_cv. Identical outputs on boot and cv, and now returns valid results with no NAs using adaptive_cv.

zachmayer commented 9 years ago

Did you test with adaptive_boot?  Could you add the tests you used to the unit tests?

Thanks for the PR, I'll review this weekend.

zachmayer commented 9 years ago

This looks good to me. Thanks for the PR.

rlesca01 commented 9 years ago

No problem. I did try it with adaptive_boot and that worked fine as well. I was thinking about unit tests and I think that example data sets would need to added in build_test_data.r that testthat uses so that adaptive_cv runs, and a different dataset besides iris would be needed so models get dropped and NAs are returned.

zachmayer commented 9 years ago

Take a look at #96: I wrote some tests to use adaptive_cv, adaptive_boot, and adaptive_LGOCV. They use the iris dataset and don't run on CRAN, so it's ok if they take a while. If you'd like to add another set of tests using a different dataset, just copy the test structure, change the X/Y variables, and submit a new PR!