zachmayer / caretEnsemble

caret models all the way down :turtle:
Other
226 stars 75 forks source link

Robust caret list #138

Closed jknowles closed 9 years ago

jknowles commented 9 years ago

I made a minor tweak to the caretList function to return NULL when do.call(train, match.args) fails instead of leaving an error. This was causing some bugs for me in a production function that called caretList from within it. This remedies that problem.

I also cleaned up the unit tests to try to fix some of the new issues and get the build to pass. It's not quite there, but I managed to track down some problems. I mistakenly rebuilt all the documentation and it incremented everything to Roxygen 4.1.1 which makes the number of files changed way bigger.

zachmayer commented 9 years ago

Could we resurrect the old continue_on_fail parameter, with a default of FALSE?

So when a user mis-specifies a single model, it will by default error out, but in a production environment where a few model failures is acceptable, you can set the parameter to TRUE and return a NULL model?

jknowles commented 9 years ago

Yeah, that makes a lot of sense. I'll code that up. Do you want to call it continue_on_fail?

zachmayer commented 9 years ago

If you think of a better name, please use it! That's just the first thing I could think of off the top of my head.

coveralls commented 9 years ago

Coverage Status

Changes Unknown when pulling fd6250afb4f70f5e5698ed55a4dc6843bc64fdc0 on jknowles:robustCaretList into \ on zachmayer:master**.

zachmayer commented 9 years ago

Also it looks like one of your skip_on_cran() tests is failing: https://travis-ci.org/zachmayer/caretEnsemble/jobs/62135683

Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  13: FUN(X[[i]], ...)
  14: predict(x, type = "prob", ...)
  15: predict(x, type = "prob", ...)
  16: predict.train(x, type = "prob", ...)
  17: extractProb(list(object), unkX = newdata, unkOnly = TRUE, ...)

  4. Failure(@test-optimizers.R#113): Warnings and fallbacks in degenerate cases -
  wghts2 not equal to c(39, 1, 60, 0)
  Mean relative difference: 0.14
zachmayer commented 9 years ago

If you fix the tests (it looks like just a couple test failures) I'll merge.

I've got a PR right behind this one to add lintr support =D

jknowles commented 9 years ago

@zachmayer I'll check into it this afternoon. I have flipped back and forth on that test a few times, so I'm considering striking it -- I'm not sure why its results vary between machines (passes on my Windows box).

zachmayer commented 9 years ago

@jknowles I'd be fine whacking it for now, and seeing if we can come up with a better version later.

coveralls commented 9 years ago

Coverage Status

Changes Unknown when pulling 868e3fcf3f734e5465800ea523f2731e66fcc2a4 on jknowles:robustCaretList into \ on zachmayer:master**.

jknowles commented 9 years ago

Almost there -- lintr support will be cool. I like the contributor guidelines as well.

jknowles commented 9 years ago

Ruh-roh. It looks like how train handles NA values in the predictors has changed. I don't think train previously failed upon NA values in the predictor matrix X, but now it appears to. This isn't a major problem, but we should cut out these tests and make a note of this issue.

zachmayer commented 9 years ago

Sounds good to me.

coveralls commented 9 years ago

Coverage Status

Changes Unknown when pulling f261bd10979d4cf8f10751340c11928b27214907 on jknowles:robustCaretList into \ on zachmayer:master**.

coveralls commented 9 years ago

Coverage Status

Changes Unknown when pulling cc7c356c79261bba35ac77e2ecbe75cdbe4ce349 on jknowles:robustCaretList into \ on zachmayer:master**.

jknowles commented 9 years ago

@zachmayer No idea why test coverage jumped so much, but I tossed all the failing NA tests.

zachmayer commented 9 years ago

@jknowles I think the first one is for not_cran = FALSE and the second is for not_cran = TRUE