rsquaredacademy / olsrr

Tools for developing OLS regression models
https://olsrr.rsquaredacademy.com/
Other
102 stars 22 forks source link

using ols_step_all_possible with Model created from dynamic function leads to "Error in eval(model$call$data) . . . not found" #176

Closed mwm-cbus closed 3 years ago

mwm-cbus commented 3 years ago

I think this package is really great,
but there is is a piece of the data access inside it that would work in more cases with an easy improvement :-)

We can't use functions such as
ols_step_all_possible
if we make our linear model in a function rather than key in everything in the same scope
because the ols function tries to reference the data frame variable named in the convenience value $call$data .

This is somewhat similar to an existing issue,
and you already identified the fix with using the real data in [model]$model:
https://github.com/rsquaredacademy/olsrr/issues/158#issuecomment-622481791

Here is a reprex:


#--- some data (thanks, datapasta !) ---

  #-- I don't think the nature of the data is significant
  #     but this is what I'm actually running --

myOrganizedDataFrame <- data.frame(
                                ADV = c(374.27,408.5,414.31,448.42,517.88,
                                        637.6,635.72,446.86,489.59,500.56,
                                        484.18,618.07,453.39,440.86,487.79,
                                        537.67,612.21,601.46,585.1,524.56,
                                        535.17,486.03,540.17,583.85,499.15),
                              BONUS = c(230.98,236.28,271.57,291.2,282.17,
                                        321.16,294.32,305.69,238.41,271.38,
                                        332.64,261.8,235.63,249.68,232.99,
                                        272.2,266.64,277.44,312.35,292.87,
                                        268.27,309.85,291.03,289.29,272.55),
                             MKTSHR = c(33L,29L,34L,24L,32L,29L,28L,
                                        31L,20L,30L,25L,34L,42L,28L,28L,
                                        30L,29L,32L,36L,34L,31L,32L,28L,
                                        27L,26L),
                             COMPET = c(202.22,252.77,293.22,202.22,
                                        303.33,353.88,374.11,404.44,394.33,
                                        303.33,333.66,353.88,262.88,333.66,
                                        232.55,273,323.55,404.44,283.11,222.44,
                                        283.11,242.66,333.66,313.44,374.11),
                              SALES = c(963.5,893,1057.25,1183.25,1419.5,
                                        1547.75,1580,1071.5,1078.25,1122.5,
                                        1304.75,1552.25,1040,1045.25,
                                        1102.25,1225.25,1508,1564.25,1634.75,
                                        1159.25,1202.75,1294.25,1467.5,1583.75,
                                        1124.75)
                       )

# ---------------------------------------------------.
#--- I use a function to make the Model from lm() ---
# ---------------------------------------------------.

multipleRegression_basicLinearModel_given_depVarLastDataframe <- function( DF_in ) {

  #--- in this function, we assume the columns are ordered as desired, 
  #                           first to last, but with dependent var LAST ---
  lastCol = length(DF_in)

  formulaString <- paste( colnames(DF_in)[[lastCol]] , " ~ ", sep="" )

  for (i in 1:(length(DF_in)-1) ) {

    nextColName = colnames(DF_in)[[i]]

    formulaString <- paste( formulaString, nextColName , sep="" )

    if ( (i+1) <= (length(DF_in)-1) ) {
      formulaString <- paste( formulaString, " + ", sep="" ) 
    }

  }

  myLinearModel = lm( as.formula(formulaString) , data=DF_in)   

  return(myLinearModel)

}

myNextModel = multipleRegression_basicLinearModel_given_depVarLastDataframe(myOrganizedDataFrame)

#--- now the data frame object DF_in in model$call$data is out of scope . . .
# . . . so the All-Possible-Regressions function fails
olsrr::ols_step_all_possible(myNextModel)

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
#
# >>> So could we please use the real values in the model 
#         captured in :
#                $model   for data
#
#         or something similar :-)
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
aravindhebbali commented 3 years ago

Hi @mwm-cbus,

We have implemented the fix here. Please download the development version of olsrr from GitHub and try again. Let me know if you run into any issues.

mwm-cbus commented 3 years ago

Hi @mwm-cbus,

We have implemented the fix here. Please download the development version of olsrr from GitHub and try again. Let me know if you run into any issues.

Thank you!

It looks great. So I've run not only ols_step_all_possible, but the couple other I'm focusing on, to include

and they all seem to be using the fix.

Thank you, and thanks again for this useful package!!