py-econometrics / pyfixest

Fast High-Dimensional Fixed Effects Regression in Python following fixest-syntax
https://py-econometrics.github.io/pyfixest/
MIT License
179 stars 35 forks source link

Add bugbear to pre-commit and some code improvements #694

Closed juanitorduz closed 3 weeks ago

juanitorduz commented 3 weeks ago

Bugbear (https://docs.astral.sh/ruff/rules/#flake8-bugbear-b) is a great tool for detecting places where bugs can appear. This PR suggests adding this to the code base. On the way I found many places where we had mutable defaults in functions, which are very dangerous https://florimond.dev/en/posts/2018/08/python-mutable-defaults-are-the-source-of-all-evil

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 78.37838% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pyfixest/estimation/feols_.py 55.55% 4 Missing :warning:
pyfixest/utils/dgps.py 0.00% 4 Missing :warning:
Flag Coverage Δ
core-tests 77.80% <78.37%> (+0.11%) :arrow_up:

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

Files with missing lines Coverage Δ
pyfixest/estimation/FixestMulti_.py 80.20% <100.00%> (ø)
pyfixest/estimation/FormulaParser.py 96.48% <ø> (-0.02%) :arrow_down:
pyfixest/estimation/estimation.py 96.15% <100.00%> (+0.27%) :arrow_up:
pyfixest/estimation/feiv_.py 98.16% <100.00%> (ø)
pyfixest/estimation/fepois_.py 87.20% <100.00%> (ø)
pyfixest/report/summarize.py 87.00% <100.00%> (+0.21%) :arrow_up:
pyfixest/estimation/feols_.py 83.24% <55.55%> (+0.25%) :arrow_up:
pyfixest/utils/dgps.py 6.30% <0.00%> (-0.24%) :arrow_down:
s3alfisc commented 3 weeks ago

Looks great, thank you Juan! I keep on being amazed by the Python tooling that is out there =) Everything looks good based on a first casual glance, though I will take a second look tomorrow before merging (I do not really trust myself after 11:00 😄 ).

juanitorduz commented 3 weeks ago

Sure! Take a detailed review, as some of the changes were made by the pre-commit hook (no rush to merge).

I'll continue adding more guard rails to make the code even better than it is 💪

s3alfisc commented 3 weeks ago

Looks all good to me! Looking forward to more guardrails =) Thanks Juan @juanitorduz!