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

BUG: Adjust eval environment for stack depth #9047

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

Conversation

rob-sil
Copy link

@rob-sil rob-sil commented Oct 28, 2023

This PR fixes the formula environment for the conditional logit and Poisson, the proportional hazards model, GEE, and QIF.

@bashtage
Copy link
Member

Overall PR looks good. Do you know if other formulas suffer, or is this the full extent fo the problem?

@bashtage
Copy link
Member

There are a few linting issues that need to be fixed. You can ignore MICE errors (needs a deep dive).

@rob-sil
Copy link
Author

rob-sil commented Oct 29, 2023

The problem appears in several other from_formulas, but gets more complicated because multiple formulas are used. For example, BetaModel takes both the main formula and an optional exog_precision_formula. That second formula isn't parsed with an eval environment, so I expect that it would similarly grab the environment from a statsmodels file rather than the user's environment.

I was trying to sidestep those cases for my first contribution (sorry about the linting error) but I just noticed GEE.from_formula has an optional second formula too. If you don't mind a longer PR, I can extend the fix to those other implementations and their other formulas.

What do you think about putting eval_env into the method signatures directly? I'm not sure if it would have to be a keyword-only argument to avoid breaking possible uses.

@josef-pkt josef-pkt added this to the 0.15 milestone Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Cox Model from formula doesn't grab the correct environment
3 participants