search

jaredlander / coefplot

Plotting model coefficients
Other
27 stars 19 forks source link

`funs()` function, deprecated in `{dplyr}` 0.8.0, has been modified so that it is not used and warning messages are not displayed. #27

Open indenkun opened 2 years ago

indenkun commented 2 years ago

If I specify anything other than identity for the trans argument in coefplot(), I get the following warning message.

library(coefplot)

model2 <- glm(price > 10000 ~ carat + cut*color, data=diamonds, family=binomial(link="logit"))
coefplot(model2, trans=invlogit)

    ## Warning: `funs()` was deprecated in dplyr 0.8.0.
    ## Please use a list of either functions or lambdas: 
    ## 
    ##   # Simple named list: 
    ##   list(mean = mean, median = median)
    ## 
    ##   # Auto named with `tibble::lst()`: 
    ##   tibble::lst(mean, median)
    ## 
    ##   # Using lambdas
    ##   list(~ mean(., trim = .2), ~ median(., na.rm = TRUE))
    ## This warning is displayed once every 8 hours.
    ## Call `lifecycle::last_lifecycle_warnings()` to see where this warning was generated.

# Figure omitted.

As you can see in the message, this is due to the use of funs(), which has been deprecated since {dplyr} 0.8.0. Currently there is no problem with the result, but it is hoped that this will be corrected in the future, as deprecated functions may be removed in the future.

The code I pulled is expected to work as before without any warning messages. If you do not like it, please discard it.

indenkun commented 2 years ago

I think it is better to replace mutate_at() with mutate(acrros(...)).

I will write and Pull Request if it is ok with the code replaced with mutate(across(...)).

jaredlander commented 2 years ago

I would love that, thanks

indenkun commented 2 years ago

Thank you for your reply.

Replaced mutate_at() with mutate(across(...)). It should return the same results as before.

indenkun commented 2 years ago

Speaking of which, because across() is a {dplyr} 1.0.0 or later function, can I rewrite the version specification in DESCRIPTION?

jaredlander commented 2 years ago

Oh, that brings up a good point. I have very mixed feelings about forcing a recent version of {dplyr}, or any package, on users. That can be very disruptive. I have to think if that's the best way forward, or if we should stick with mutate_at().

I know you put a lot of effort into rewriting this PR so I don't want that to go to waste, but I think it's important to be conscientious about this.

indenkun commented 2 years ago

Oh, I thought I rewrote buildModelCI.default() (https://github.com/jaredlander/coefplot/pull/27/commits/9cf1590a594d95eb6b0026d950b6f6c9fcbfac1e), but did you not receive it? Was it incomplete? Did I do something wrong in my use of GitHub?

Or maybe you also expect the rewrite of mutate_at() in the other part (extract.coef.xgb.Booster())? Then do you also expect to rewrite filter_at and other *_at types?

The rewriting doesn't seem to be that hard, but I think it will take some time to test because some of the expected results are not clear.

Also, is it correct that DESCRIPTION should not be rewritten?

I am a non-native speaker of English, so I apologize if I have misinterpreted your meaning.