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

Drop ggpubr dependency #215

Merged
merged 6 commits into from
Nov 13, 2024
Merged

Drop ggpubr dependency #215

merged 6 commits into from
Nov 13, 2024

Conversation

grantmcdermott
Copy link
Contributor

Closes #210.

This PR simply drops ggpubr and replaces one or two functions with with "native" ggplot2 equivalents.

The plots are slightly different, but I personally doubt that anyone will really notice. More importantly, this small change leads to over 30(!) fewer dependencies.

pkgload::load_all("~/Documents/Projects/did-1/")
#> ℹ Loading did
data(mpdta)

out <- att_gt(
  yname = "lemp",
  gname = "first.treat",
  idname = "countyreal",
  tname = "year",
  xformla = ~1,
  data = mpdta,
  est_method = "reg"
)
ggdid(out, ylim = c(-.25, .1))

es <- aggte(out, type = "dynamic")
ggdid(es)

Created on 2024-11-04 with reprex v2.1.1

PS. If you want to continue down this road, then there are more places for trimming. E.g. If you already have data.table, then you probably get away with importing dplyr and tidyr. But I'll leave that for you to signal interest.

@pedrohcgs
Copy link
Collaborator

I will check this in a bit. But @grantmcdermott, you know the automatic tests are not passing.... =P

@grantmcdermott
Copy link
Contributor Author

I will check this in a bit. But @grantmcdermott, you know the automatic tests are not passing.... =P

Well, the R CMD tests are all passing. It's the mysterious "R-Package-Test" workflow that's failing...

More seriously, I think it's just a minor undeclared dependency issue. We just need to add broom (or even backports) to Suggests. Let me try that and then retrigger the test suite.

@grantmcdermott
Copy link
Contributor Author

grantmcdermott commented Nov 13, 2024

Ugh, looks like I made things worse. Serves me right for doing this on my phone before getting the kids to school. I'll take a proper look later and ping you when it's ready.

@pedrohcgs
Copy link
Collaborator

Sounds good!!!

@pedrohcgs
Copy link
Collaborator

I think it may be the data.table because I merged a PR that updated the code and leverages the data.table now.

@pedrohcgs
Copy link
Collaborator

To be clear, the tests are also passing locally here

@pedrohcgs
Copy link
Collaborator

We are good! It is not passing!

@pedrohcgs pedrohcgs merged commit 6a951f2 into bcallaway11:master Nov 13, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce dependencies
2 participants