Skip to content

Prevent Casting Dependent Variable to Integer to Avoid Information Loss in Poisson Regression#900

Merged
s3alfisc merged 4 commits intopy-econometrics:masterfrom
JaapCTJ:remove-casting-to-integer-in-fepois
May 12, 2025
Merged

Prevent Casting Dependent Variable to Integer to Avoid Information Loss in Poisson Regression#900
s3alfisc merged 4 commits intopy-econometrics:masterfrom
JaapCTJ:remove-casting-to-integer-in-fepois

Conversation

@JaapCTJ
Copy link
Copy Markdown
Contributor

@JaapCTJ JaapCTJ commented May 12, 2025

This pull request removes the line of code in fepois_.py that converts the dependent variable Y to an integer. Poisson regression can be applied to any non-negative outcomes, and there is no need to limit its use to non-negative integer outcomes by converting float Y values into integers.

This makes the results more consistent with the Fixest package in R, where it does not perform this conversion either.

Additionally, it might be nice to add a non-negative non-integer generated dependant variable in the get_data() function in utils.py. This would demonstrate the versatility of Poisson regression beyond integer-only outcomes and provide users with a broader perspective on its application.

@s3alfisc
Copy link
Copy Markdown
Member

pre-commit.ci autofix

@JaapCTJ
Copy link
Copy Markdown
Contributor Author

JaapCTJ commented May 12, 2025

Instead of removing the _to_integer() function entirely, I changed it to _check_series_or_dataframe() to keep the essential check that the outcome variable should be a pandas series or dataframe.

@codecov
Copy link
Copy Markdown

codecov bot commented May 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
core-tests 80.38% <100.00%> (+0.03%) ⬆️
tests-extended ?
tests-vs-r 47.09% <100.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
pyfixest/estimation/fepois_.py 90.04% <100.00%> (ø)
pyfixest/utils/dev_utils.py 86.76% <100.00%> (+2.98%) ⬆️

... and 6 files with indirect coverage changes

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

@s3alfisc
Copy link
Copy Markdown
Member

Looks good to me! Thank you Jaap @JaapCTJ

@s3alfisc s3alfisc merged commit adfff43 into py-econometrics:master May 12, 2025
9 checks passed
@s3alfisc
Copy link
Copy Markdown
Member

@all-contributors please add @JaapCTJ for code

@allcontributors
Copy link
Copy Markdown
Contributor

@s3alfisc

I've put up a pull request to add @JaapCTJ! 🎉

damandhaliwal pushed a commit to damandhaliwal/pyfixest that referenced this pull request Jun 17, 2025
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.

2 participants