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

add caret tidiers #344

Merged
merged 11 commits into from Jun 12, 2018

Conversation

Projects
None yet
3 participants
@mkuehn10
Contributor

mkuehn10 commented Jun 10, 2018

implements #270

@mkuehn10

This comment has been minimized.

Contributor

mkuehn10 commented Jun 10, 2018

Failure message was
Packages suggested but not available: 'gamlss' 'ks' 'rgeos' The suggested packages are required for a complete check. Checking can be attempted without them by setting the environment variable _R_CHECK_FORCE_SUGGESTS_ to a false value. See section 'The DESCRIPTION file' in the 'Writing R Extensions' manual.

I don't think my PR has anything to do with those packages, I added caret to the DESCRIPTION for Suggests.

@alexpghayes

Looks useful, can't wait to give it a go myself! Requested some small changes.

Not sure why the build is failing, will have to look into that next week.

#' \code{glance} method.
#'
#' @param x An object of class \code{confusionMatrix}
#' @param show_class A boolean of whether to show the values for class specific

This comment has been minimized.

@alexpghayes

alexpghayes Jun 10, 2018

Collaborator

I'd use "logical" instead of boolean since that's more standard for R.

This comment has been minimized.

@alexpghayes

alexpghayes Jun 10, 2018

Collaborator

I'd also rename show_class to by_class.

#' to FALSE, result will only show accuracy and kappa.
#' @param ... extra arguments (not used)
#'
#' @return A data.frame with one or more of the following columns:

This comment has been minimized.

@alexpghayes

alexpghayes Jun 10, 2018

Collaborator

Should return a tibble!

This comment has been minimized.

@mkuehn10

mkuehn10 Jun 10, 2018

Contributor

I think I need to fix the durbinWatsonTest as well then.

@alexpghayes

This comment has been minimized.

Collaborator

alexpghayes commented Jun 10, 2018

This looks good! I'd like to merge but I need to think a little bit about the implications of adding caret to Suggests first. I'll get back to you soon!

@mkuehn10

This comment has been minimized.

Contributor

mkuehn10 commented Jun 10, 2018

It's giving me errors on my test script saying that str_to_lower couldn't be found, but it's from the stringr package.

@atyre2

This comment has been minimized.

Contributor

atyre2 commented Jun 11, 2018

WRT to builds failing on travis-ci, this message is at the end of all the failures I"ve just checked:
The job exceeded the maximum time limit for jobs, and has been terminated.
It looks like it is taking a very long time to get the machine set up, so never gets around to testing the package.

@alexpghayes

This comment has been minimized.

Collaborator

alexpghayes commented Jun 11, 2018

At the moment it's okay for the Travis build to fail, that's on us, but AppVeyor does need to pass. Pretty busy the next couple of days but will get back to this at some point this week.

@mkuehn10

This comment has been minimized.

Contributor

mkuehn10 commented Jun 11, 2018

Thanks @atyre2 I saw that as well. Thanks for the response @alexpghayes, I'll do whatever is needed to fix this, but I'm really not sure what's going on.

mkuehn10 added some commits Jun 11, 2018

@alexpghayes alexpghayes merged commit bc29e0c into tidymodels:master Jun 12, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@alexpghayes

This comment has been minimized.

Collaborator

alexpghayes commented Jun 12, 2018

Awesome! Thanks for your persistence with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment