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

Export more S3 methods for model_parameters() generic #746

Open
IndrajeetPatil opened this issue Jul 17, 2022 · 8 comments
Open

Export more S3 methods for model_parameters() generic #746

IndrajeetPatil opened this issue Jul 17, 2022 · 8 comments
Labels
Beginner-friendly 🤝 Friendly for new contributors Low priority 😴 This issue does not impact package functionality much

Comments

@IndrajeetPatil
Copy link
Member

A lot of the objects listed below are actually supported in parameters, but developers who will be comparing it with broom will think they aren't because there are no exported methods in NAMESPACE.

broom_tidy <- ls(getNamespace("broom"))
broom_tidy <- broom_tidy[grepl("^tidy", broom_tidy)]
broom_supported <- gsub("^tidy.", "", broom_tidy)
easystats_tidy <- ls(getNamespace("parameters"))
easystats_tidy <- easystats_tidy[grepl("^model_parameters", easystats_tidy)]
easystats_supported <- gsub("^model_parameters.", "", easystats_tidy)

broom_supported[!broom_supported %in% easystats_supported]
#>   [1] "emmeans"                  "emmeans_summary"         
#>   [3] "irlba"                    "optim"                   
#>   [5] "svd"                      "xyz"                     
#>   [7] "aareg"                    "acf"                     
#>   [9] "Arima"                    "biglm"                   
#>  [11] "binDesign"                "binWidth"                
#>  [13] "boot"                     "btergm"                  
#>  [15] "cch"                      "character"               
#>  [17] "cld"                      "clm"                     
#>  [19] "confint.glht"             "confusionMatrix"         
#>  [21] "coxph"                    "crr"                     
#>  [23] "cv.glmnet"                "density"                 
#>  [25] "dgCMatrix"                "dgTMatrix"               
#>  [27] "dist"                     "drc"                     
#>  [29] "durbinWatsonTest"         "ergm"                    
#>  [31] "factanal"                 "felm"                    
#>  [33] "fixest"                   "ftable"                  
#>  [35] "garch"                    "geeglm"                  
#>  [37] "glmnet"                   "glmrob"                  
#>  [39] "glmRob"                   "gmm"                     
#>  [41] "ivreg"                    "kappa"                   
#>  [43] "kde"                      "Kendall"                 
#>  [45] "leveneTest"               "Line"                    
#>  [47] "Lines"                    "lm"                      
#>  [49] "lm.beta"                  "lmrob"                   
#>  [51] "lmRob"                    "logical"                 
#>  [53] "lsmobj"                   "manova"                  
#>  [55] "map"                      "mfx"                     
#>  [57] "mlogit"                   "muhaz"                   
#>  [59] "nlrq"                     "nls"                     
#>  [61] "NULL"                     "numeric"                 
#>  [63] "orcutt"                   "plm"                     
#>  [65] "poLCA"                    "Polygon"                 
#>  [67] "Polygons"                 "power.htest"             
#>  [69] "prcomp"                   "pyears"                  
#>  [71] "rcorr"                    "ref.grid"                
#>  [73] "regsubsets"               "rlm"                     
#>  [75] "roc"                      "rq"                      
#>  [77] "sarlm"                    "Sarlm"                   
#>  [79] "sparseMatrix"             "SpatialLinesDataFrame"   
#>  [81] "SpatialPolygons"          "SpatialPolygonsDataFrame"
#>  [83] "spec"                     "speedglm"                
#>  [85] "speedlm"                  "summary_emm"             
#>  [87] "summary.glht"             "summary.lm"              
#>  [89] "summary.plm"              "summaryDefault"          
#>  [91] "survdiff"                 "survexp"                 
#>  [93] "survfit"                  "survreg"                 
#>  [95] "svyolr"                   "table"                   
#>  [97] "tobit"                    "ts"                      
#>  [99] "TukeyHSD"                 "zoo"

Created on 2022-07-17 by the reprex package (v2.0.1)

This is because most of them are supported by the default method:

library(parameters)
library(speedglm)
#> Loading required package: Matrix
#> Loading required package: MASS

sloop::s3_dispatch(model_parameters(lm(wt ~ mpg, mtcars)))
#>    model_parameters.lm
#> => model_parameters.default

Created on 2022-07-17 by the reprex package (v2.0.1)

I think we just need to do something like-

#' @export
model_parameters.lm <- model_parameters.default

This is low priority, and probably easy enough to be handled by newcomers.

@IndrajeetPatil IndrajeetPatil added Low priority 😴 This issue does not impact package functionality much Beginner-friendly 🤝 Friendly for new contributors labels Jul 17, 2022
@bwiernik
Copy link
Contributor

If someone implements this, all of the assignments should be stacked like:

model_parameters.a <- 
  model_parameters.b <-
  model_parameteters.default

@IndrajeetPatil
Copy link
Member Author

But, IINM, model_parameters.b won't appear in NAMESPACE if we do it this way.

We'll need to do something like this if both of them are to appear in NAMESPACE:

#' @export
model_parameters.a <- model_parameteters.default

#' @export
model_parameters.b <- model_parameteters.default  

@IndrajeetPatil
Copy link
Member Author

But, IINM, model_parameters.b won't appear in NAMESPACE if we do it this way.

Yes, I can confirm that this is the case.
Just tried it with the code below, and the middle ones in the chain disappear from NAMESPACE:

#' @export
ci.rq <- ci.default
#' @export
ci.rqss <- ci.default
#' @export
ci.crq <- ci.default
#' @export
ci.nlrq <- ci.default
#' @export
ci.rqs <- ci.default

@bwiernik
Copy link
Contributor

bwiernik commented Jul 17, 2022

Hmmm, there's a way, let me see what I did before

Edit: ah nope, I was misremembering.

@IndrajeetPatil
Copy link
Member Author

Okay. Thanks for checking!

That would have indeed been cleaner if it were supported. Is it that roxygen2 can't handle this, or that R itself doesn't support it?

Just wondering if we can make a feature request.

@IndrajeetPatil
Copy link
Member Author

But, regardless, at least for now we can just do the following:

#' @export
model_parameters.a <- model_parameteters.default

#' @export
model_parameters.b <- model_parameteters.default  

@bwiernik
Copy link
Contributor

It's a Roxygen limitation

@jimrothstein
Copy link
Contributor

In a PR, I the added export S3 methods for model_parameters() generic:

Just as FYI reference, here is clumsy code I used:

missing classes

x <- broom_supported[!broom_supported %in% easystats_supported]

and to construct this

#' @export
model_parameters.CLASS <- model_parameters.default

use clumsy code

cat(vapply(x,
function(x) { paste0("#' @export\n", "model_parameters.",x, " <- model_parameters.default\n\n")},
character(length = 1),
USE.NAMES=F)

)

ALSO, I found this *without* function a little easier to decipher:

"%w/o%" <- function(x, y) x[!x %in% y] #-- x without y
x <- broom_supported %w/o% easystats_supported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Beginner-friendly 🤝 Friendly for new contributors Low priority 😴 This issue does not impact package functionality much
Projects
None yet
Development

No branches or pull requests

3 participants