Skip to content

Create DID: Prettier and Unified Coefficient Names for DiD Methods #838

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

asteves
Copy link
Contributor

@asteves asteves commented Mar 11, 2025

PR to fix #407

  • Creates a utility function for renaming coefficients
  • Updates tidy() and summary() methods in DiD class methods

Copy link

codecov bot commented Mar 11, 2025

Codecov Report

Attention: Patch coverage is 64.91228% with 20 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pyfixest/did/did2s.py 14.28% 6 Missing ⚠️
pyfixest/did/twfe.py 14.28% 6 Missing ⚠️
pyfixest/utils/utils.py 87.87% 4 Missing ⚠️
pyfixest/did/lpdid.py 57.14% 3 Missing ⚠️
pyfixest/estimation/feols_.py 66.66% 1 Missing ⚠️
Flag Coverage Δ
core-tests 80.28% <64.91%> (-0.11%) ⬇️
tests-extended ?
tests-vs-r 47.88% <19.29%> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pyfixest/estimation/feols_.py 83.31% <66.66%> (-6.98%) ⬇️
pyfixest/did/lpdid.py 92.85% <57.14%> (-1.77%) ⬇️
pyfixest/utils/utils.py 91.24% <87.87%> (-1.07%) ⬇️
pyfixest/did/did2s.py 87.38% <14.28%> (-3.18%) ⬇️
pyfixest/did/twfe.py 70.96% <14.28%> (-9.81%) ⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@asteves asteves marked this pull request as draft March 11, 2025 16:37
@s3alfisc
Copy link
Member

s3alfisc commented Mar 12, 2025

Hi @asteves , thanks for taking a stab at this! Somewhere down in the codebase I had a rename_event_study_coefficients function but it seems no longer functional, so this PR is super welcome =) Is it ready for me to take a first look? image

The above returns
image

but the coef names should be var::time as in the lpdid method.

@asteves
Copy link
Contributor Author

asteves commented Mar 12, 2025

About a year late from when I grabbed it. @s3alfisc I think it's good for a first look.

@s3alfisc
Copy link
Member

Hi @asteves , I unfortunately will not manage to take a look tonight, will try my best tomorrow evening!

@asteves asteves marked this pull request as ready for review April 8, 2025 14:44
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.

DID: Prettier and Unified Coefficient Names for DiD Methods
2 participants