search

zachmayer / caretEnsemble

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

Added as.caretList function #211

Closed washcycle closed 7 years ago

washcycle commented 7 years ago

Added ability to convert list of caret models into a caretList object.

lintr-bot commented 7 years ago

R/caretList.R:263:24: style: Only use double-quotes.

​  if(class(modelList)!='list'){
                       ^~~~~~

R/caretList.R:267:23: style: Only use double-quotes.

​  class(modelList) <- 'caretList'
                      ^~~~~~~~~~~
zachmayer commented 7 years ago

I don't like is.caretList changing the object's type.

I'd prefer a new as.caretList function.

Sent from my iPhone

On Sep 7, 2016, at 11:37 PM, Matt notifications@github.com wrote:

Added ability to convert list of caret models into a caretList object.

You can view, comment on, or merge this pull request online at:

https://github.com/zachmayer/caretEnsemble/pull/211

Commit Summary

Added as.caretList function File Changes

M R/caretList.R (12) Patch Links:

https://github.com/zachmayer/caretEnsemble/pull/211.patch https://github.com/zachmayer/caretEnsemble/pull/211.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

washcycle commented 7 years ago

Hmmm this is an as.caretList function? Did I misunderstand your comment? The reason I wanted this was because I trained several models and one of them didn't work out. I wanted to drop it.

I could subset the models by models[1:3] dropping the reaming model(s). However, to ensemble them they needed to be of type caretList. I looked at how the caretList was created which led me to write this function which worked great.

zachmayer commented 7 years ago

I think we could write a [ method for caretList that doesn't drop the class on subsetting.

washcycle commented 7 years ago

I like that idea, and I saw where I goofed btw. That new function was suppose to be an as.caret function)

coveralls commented 7 years ago

Coverage Status

Coverage increased (+31.9%) to 96.315% when pulling 4c9ae412b6aa63386adf3b810b1da965815d5ea1 on washcycle:patch-1 into 27a45556876774946bd82e21d19a01a84aba35f6 on zachmayer:master.

zachmayer commented 7 years ago

2 more notes:

  1. It looks like as is an S4 method, so we should write an S4 method here. I dunno how to do that, but I'll do some research.
  2. Please add a test
washcycle commented 7 years ago

Just so we are on the same page, as is an S4 method, but as.'classname' is an S3 method. getS3method('as.data.frame', 'data.frame') I'll probably go and stop being lazy and clone and make these changes and add a test(s) this weekend. I'll make a new request once i'm done and close/references these ones out. I'll take a stab at adding the [ method too.

zachmayer commented 7 years ago

That would be perfect, thanks!

Sent from my iPhone

On Sep 8, 2016, at 7:11 PM, Matt notifications@github.com wrote:

As is an S4 method, but as.classname is an S3 method. getS3method('as.data.frame', 'data.frame') I'll probably go and stop being lazy and clone and make these changes and add a test(s) this weekend. I'll make a new request once i'm done and close/references these ones out.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.