Skip to content

fix: VarLookupDict __iter__ + DataFrame dtype compat (fixes #259, #232)#260

Open
kouyouqi123 wants to merge 3 commits into
pydata:masterfrom
kouyouqi123:master
Open

fix: VarLookupDict __iter__ + DataFrame dtype compat (fixes #259, #232)#260
kouyouqi123 wants to merge 3 commits into
pydata:masterfrom
kouyouqi123:master

Conversation

@kouyouqi123
Copy link
Copy Markdown

@kouyouqi123 kouyouqi123 commented May 31, 2026

Fixes #259 — VarLookupDict missing __iter__

Added __iter__, __len__, and keys() to VarLookupDict to prevent CPython crash during traceback formatting.

Fixes #232 — DataFrame .dtype access in _eval_factor

Changed result.dtype to np.asarray(result).dtype so the error message works with both numpy arrays and pandas DataFrames.

VarLookupDict is a dict-like object but does not implement __iter__,
__len__, or keys(). This causes a CPython crash (via a KeyError: 0)
when Python's C-level error display code tries to iterate over the
locals dict during traceback formatting.

Fixes pydata#259
…me compat

When _eval_factor receives a pandas DataFrame as result and
tries to raise a PatsyError, it accesses result.dtype which
doesn't exist on DataFrames (only .dtypes). This causes a
confusing secondary AttributeError instead of the intended
error message.

Fixes pydata#232
@kouyouqi123 kouyouqi123 changed the title fix: implement __iter__ on VarLookupDict to prevent CPython crash fix: VarLookupDict __iter__ + DataFrame dtype compat (fixes #259, #232) May 31, 2026
@bashtage
Copy link
Copy Markdown
Contributor

This is the type of change that is considered hard in patsy. Could you explain more as to why this is experienced when it does not come up in standard CPython?

@bashtage
Copy link
Copy Markdown
Contributor

Also, does this happen in standard CPython, only not get tested in patsy?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 31, 2026

Codecov Report

❌ Patch coverage is 27.27273% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.20%. Comparing base (331879f) to head (128b51d).
⚠️ Report is 60 commits behind head on master.

Files with missing lines Patch % Lines
patsy/eval.py 27.27% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
+ Coverage   97.60%   98.20%   +0.59%     
==========================================
  Files          30       30              
  Lines        3096     3114      +18     
  Branches      591      679      +88     
==========================================
+ Hits         3022     3058      +36     
+ Misses         39       35       -4     
+ Partials       35       21      -14     
Flag Coverage Δ
unittests 97.81% <27.27%> (+0.20%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@bashtage
Copy link
Copy Markdown
Contributor

I think I understand this issues now. Shouldn't the fix either go into the notebook environment or into CPython directly? These appear to be fully upstream of patsy.

The hesitation is around the general treatment of patsy in being in maintenance mode. This has been interpreted to mean fixes for change in CPython where essential for it to run.

This said, statsmodels need to get out the version where formulaic is available.

@bashtage
Copy link
Copy Markdown
Contributor

For this to have a realistic prospect (given the status of the bug and the maintenance mode of patsy), we will need 100% coverage. Ideally you could add a single test that triggers the error using the same API that caused this issue in the first place, and we will see that everything that has been added will be hit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants