search

paulvanderlaken / ppsr

R implementation of Predictive Power Score
GNU General Public License v3.0
74 stars 9 forks source link

Error: For a classification model, the outcome should be a factor. #11

Closed laresbernardo closed 3 years ago

laresbernardo commented 3 years ago

Great package, thanks for sharing @paulvanderlaken I had my eye over this Predictive Power Score methodology for a while now and glad you did this! I am trying to use the package as a plug-and-play function using Titanic dataset but am getting the following error which I think you can avoid by converting character classes into factors in the backend when detected. Is that correct?

> pp <- ppsr::visualize_predictors(dft, "Survived")
Error: For a classification model, the outcome should be a factor.

> rlang::last_error()
<error/rlang_error>
For a classification model, the outcome should be a factor.
Backtrace:
 1. ppsr::visualize_predictors(dft, "Survived")
 2. ppsr::score_predictors(df, y)
 3. ppsr::score(x = df[[x]], y = df[[y]], ...)
 5. parsnip::fit.model_spec(model, formula = y ~ x, data = df)
 6. parsnip:::form_form(object = object, control = control, env = eval_env)

I guess you've got the idea as you do something similar here.

paulvanderlaken commented 3 years ago

I have a hunch that the current target labels might be 1/2 or something like that, and that my function does not detect those as binary classes correctly. Can you share this Titanic dataset of your? Or can I download your exact version somewhere so I can reproduce this error?

paulvanderlaken commented 3 years ago

I ran into the same error developing today on a different dataset, and I think the bug is solved in the latest commit to the development branch. Could you check whether it now works as planned on your Titanic dataset?

laresbernardo commented 3 years ago

Hi @paulvanderlaken This is a reproducible example.

library(lares) #remotes::install_github("laresbernardo/lares")
data(dft)
ppsr::visualize_predictors(dft, "Survived")

In this case, Survived is a logical feature and I guess this should be enough to detect binaries, no matter their type.

paulvanderlaken commented 3 years ago

Hi Lares, thanks for notifying me of this error. I hadn't yet implemented a test for these cases. Pretty basic scenario so should definitely not result in an error! Issue is now solved in latest developmental branch v0.0.0.921 so I will close this issue. Feel free to open again if you run into any errors!