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

Passing CRAN and expanding test coverage #39

Closed
wants to merge 24 commits into from

Conversation

jknowles
Copy link
Contributor

@jknowles jknowles commented Nov 5, 2014

Close #37. Close #38. Accomplish many of the tasks on #32.

Add unit tests for all helper functions for caretEnsemble. Expand unit test coverage to smaller helper functions like weighted standard deviations. Fix summary.caretEnsemble to produce correct results for ensembled models.

Still need to make the predict.caretEnsemble output more consistent across different options.

Fixes a number of issues related to varImp, though it does not fail gracefully in the case when varImp method fails for a specific method passed to train.

Added a new test suite using more diverse train methods and some missingness. This checks against some errors relating to building to specifically to the included test models and only increases the size of the package modestly.

@@ -104,23 +105,20 @@ caretEnsemble <- function(all.models, optFUN=NULL, ...){
predict.caretEnsemble <- function(object, keepNA = TRUE, se = NULL, ...){
Copy link
Owner

Choose a reason for hiding this comment

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

I'm getting this note when I run check():
checking Rd \usage sections ... NOTE
S3 methods shown with full name in documentation object 'predict.caretEnsemble':
‘predict.caretEnsemble’

S3 methods shown with full name in documentation object 'predict.caretStack':
‘predict.caretStack’

The \usage entries for S3 methods should use the \method markup and not
their full name.
See the chapter ‘Writing R documentation files’ in the ‘Writing R
Extensions’ manual.

I think adding the @method tag to predict for caretEnsemble and caretStack fixes this:

@method predict caretEnsemble
@method predict caretStack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, that makes more sense. I will make the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why this happened under Roxygen and why I didn't see it on my machine. I"ll investigate.

Copy link
Owner

Choose a reason for hiding this comment

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

I think this is related to the S4/S3 issue we're discussing elsewhere.


#' Extract a model accuracy metric from a \code{\link{train}} object.
#' @rdname metrics
#' @export
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should also include:
@method getMetric train

This was referenced Nov 11, 2014
@zachmayer
Copy link
Owner

I'm going to close this, and we can move the discussion over to the #41, #42 and #43

@zachmayer zachmayer closed this Nov 11, 2014
@jknowles jknowles deleted the moartests branch November 11, 2014 21:55
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.

Document undocumented data sets to pass CMD CHECK
2 participants