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

method for augment or fortify. #377

Open
McCartneyAC opened this issue Feb 9, 2021 · 6 comments
Open

method for augment or fortify. #377

McCartneyAC opened this issue Feb 9, 2021 · 6 comments

Comments

@McCartneyAC
Copy link

Hi guys,

I am trying to use {estimatr} with a lot of downstream packages (such as {lindia} or {report}, or to do added variable plots with my own package) that rely on a model matrix that we would usually get from broom::augment() or ggplot2::fortify().

I know this is in your heads because I found this issue for broom::fortify():

tidymodels/broom#237

I'm just including it here as an issue because I took a first stab at trying to do it myself and got very stuck, very quickly.

@nfultz
Copy link
Contributor

nfultz commented Feb 9, 2021 via email

@McCartneyAC
Copy link
Author

I got this to work! Let me play with it a bit in my use case but this is super helpful.

I guess the next issue is just dealing with the problem of standard errors given the heteroskedasticity issue. Have to make some judgment calls. Thanks.

@lukesonnet
Copy link
Contributor

@nfultz, should we create our own augment method that requires newdata? Is that advisable?

@nfultz
Copy link
Contributor

nfultz commented Sep 7, 2021

Since augment.lm isn't exported, I think it would be better to add augment.lm_robust in the broom package instead of here:

augment.lm_robust <- function(x, data, newdata, ...) {
  # force(newdata) or similar check
  # delegate to augment.lm(x, data, newdata, ...)
}

@McCartneyAC
Copy link
Author

McCartneyAC commented Sep 16, 2021

Correct me if I'm wrong, but isn't it current practice with {broom} that they're no longer adding new model objects and things like this? or am I thinking of a different ecosystem?

EDIT: From the {broom} main website:

However, for maintainability purposes, the broom package authors now ask that requests for new methods be first directed to the parent package (i.e. the package that supplies the model object) rather than to broom. New methods will generally only be integrated into broom in the case that the requester has already asked the maintainers of the model-owning package to implement tidier methods in the parent package.

@nfultz
Copy link
Contributor

nfultz commented Sep 20, 2021

That makes sense. estimatr is a bad case, because the model objects are generally compatible with other methods for lm but not always. It doesn't have lm in it's list of classes because that's a promise of compatibility that it generally can't keep. And since augment.lm is not exported, can't explicilty opt in to using the feature either from the estimatr package namespace.

Something like the below might work, but it's ugly as sin and will probably fail somewhere down the line if/when broom relies on other methods for lm that are similarly incompatible.

augment.lm_robust <- function(x, data, newdata, ...) {
  # force(newdata) or similar check
  class(x) <- "lm"
  augment(x, data, newdata, ...)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants