Skip to content

Add WooldridgeDiD opt-out for comparison-support filtering - #820

Merged
igerber merged 2 commits into
mainfrom
codex/eval-api-a9439f07
Sep 7, 2026
Merged

Add WooldridgeDiD opt-out for comparison-support filtering#820
igerber merged 2 commits into
mainfrom
codex/eval-api-a9439f07

Conversation

@igerber

@igerber igerber commented Sep 5, 2026

Copy link
Copy Markdown
Owner

WooldridgeDiD users can now set unsupported_period_action="error" to refuse a fit when periods lack eligible comparison observations. The error identifies the unsupported periods and affected observation count before filtering or interaction construction. The default "drop" preserves existing filtering, warnings, and estimates.

The option applies to OLS, logit, and Poisson independently of rank_deficient_action. Constructor, transactional set_params(), and fit-time checks validate the policy. Results record the fit-time policy in summaries and dictionaries and retain it through aggregation and serialization.

Existing pre-filter configuration, cohort, and survey-design checks retain precedence. Later covariate, nonlinear-outcome, and some explicit-cluster errors can be preceded by support refusal. Keeping that order preserves successful default fits when invalid nonlinear outcomes occur only in discarded periods; regression tests cover this case. For valid survey designs requiring period removal, "drop" retains the existing NotImplementedError, while "error" raises the policy ValueError.

Methodology references

  • Method: Wooldridge extended two-way fixed effects (ETWFE).
  • Sources: Wooldridge (2025), Section 5.4, and Wooldridge (2023), nonlinear extensions.
  • The refusal option is a documented library policy. The comparison-support predicate is unchanged: positive-weight never-treated observations for OLS with control_group="never_treated"; other paths also admit observations before g - anticipation. Unidentified-cohort exclusion remains separate and active. Weighting, variance, inference, and default numerical behavior are unchanged.

Validation

  • Tests added/updated: 87 policy cases cover methods, control groups, rank actions, anticipation, survey validation, bootstrap/cohort trends, parameter handling, result provenance, aggregation, and serialization. These include 14 cases specifying later-validation behavior. Ledger assertions and affected documentation were updated.
  • Python: latest focused policy run: 87 passed. The three ETWFE suites previously exercised 451 distinct tests, including slow bootstrap, across the full-suite and focused runs.
  • Rust: all three ETWFE suites with slow tests enabled: 465 passed.
  • Contracts/docs: latest ledger, guides, docs structure/dependencies, tracking, and changelog suites: 725 passed, 24 skipped. Broader contract/docs checks, including base-estimator, serialization, and naming suites, also ran: 1,035 passed, 67 skipped. Skips cover optional dependencies and inapplicable generic/lifecycle probes.
  • Notebook: Python execution of 16_wooldridge_etwfe.ipynb with nbmake: 1 passed. Its five stored ETWFE summaries now show the default policy; numerical and image outputs are unchanged.
  • Static/repository checks: Ruff, Black, changelog validation, docs-impact inspection, secret-pattern scan, and git diff --check passed. The pre-merge scanner found zero patterns; its results-module test-resolution warning is covered by explicit Wooldridge and serialization tests.

Type-checking limitation: configured mypy diff_diff targets Python 3.10 and stops in installed NumPy 2.5.2 stubs, which use Python 3.12 type statements. A minimal mypy -c 'import numpy' reproduced the failure. Supplemental mypy --python-version 3.14 diff_diff passed for 108 source files; it does not replace the configured check. The local environment uses Python 3.14 and mypy 2.3.1; the lint workflow instead pins NumPy 2.4.5.

The full library suite and full Sphinx build were not run. R/Stata parity tests used committed reference artifacts; external R/Stata executables were not invoked.

Security / privacy

  • No secrets or PII added. Tests use synthetic panels and existing committed reference artifacts.

Changelog

  • Added changelog.d/20260905-wooldridge-unsupported-period-action.md, referencing ledger row M-147.

@igerber igerber added ready-for-ci Triggers CI test workflows and removed ready-for-ci Triggers CI test workflows labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Overall assessment

Looks good — no unmitigated P0 or P1 findings.

Executive summary

  • Affects Wooldridge ETWFE across OLS, logit, and Poisson.
  • Default estimation, weighting, variance, SEs, and support predicates are unchanged.
  • The opt-out is explicitly documented in the Methodology Registry and consistent with Wooldridge Section 5.4.
  • Two P2 consistency issues remain: incomplete validation precedence and stale notebook outputs.

Methodology

P3-informational — Documented library policy

  • Impact: None. "error" refuses an unidentified full-sample design rather than changing the ETWFE estimand. Default "drop" retains the Section 5.4 reduction and Stata-parity behavior.
  • Evidence: docs/methodology/REGISTRY.md:L2426-L2427, docs/methodology/papers/wooldridge-2025-review.md:L461-L477, diff_diff/wooldridge.py:L1525-L1537.
  • Concrete fix: None required.

No weighting, aggregation, variance, SE, or inference methodology changes were found.

Code Quality

P2 — “Input validation runs first” is not fully implemented

  • Impact: On an unsupported panel, "error" exits before existing nonlinear outcome, covariate, and explicit-cluster validation. For example, a missing/invalid logit outcome or missing exovar produces the comparison-support error first, contrary to the documented validation precedence.
  • Evidence: The early exit is at diff_diff/wooldridge.py:L1525-L1537, while covariates and nonlinear outcomes are validated later at diff_diff/wooldridge.py:L1895-L1905, diff_diff/wooldridge.py:L2806-L2815, and diff_diff/wooldridge.py:L3068-L3077.
  • Concrete fix: Preflight required columns and method-specific outcome validity before the support-policy gate, with tests combining unsupported periods with missing covariates, invalid outcomes, and missing cluster columns.

Performance

No findings. The default path adds only trivial validation and metadata assignment.

Maintainability

No findings. Parameter propagation through BaseEstimator, aggregation, and pickle migration is coherent.

Tech Debt

No untracked debt found. The completed TODO entry is removed and M-147 records the new policy.

Security

No findings or apparent secrets.

Documentation/Tests

P2 — Tutorial outputs retain the old summary format

  • Impact: The tutorial prose says summary() exposes unsupported_period_action, but all five committed output blocks omit the new line, so rendered documentation disagrees with runtime output.
  • Evidence: Runtime output changed at diff_diff/wooldridge_results.py:L738-L750; stale output begins at docs/tutorials/16_wooldridge_etwfe.ipynb:L248-L258.
  • Concrete fix: Re-execute the notebook and commit its outputs, or update all stored summary blocks to include Unsupported period action: drop.

The changelog fragment follows the repository convention. Independent test execution was unavailable because pytest is not installed in the review environment.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: f7afa234ef2272edfb49a7528d84e76a87222e8a


Overall assessment

Looks good — no unmitigated P0 or P1 findings.

Executive summary

  • Adds a refusal policy for Wooldridge ETWFE across OLS, logit, and Poisson.
  • Default estimates, weighting, variance, SEs, and inference remain unchanged.
  • The policy is documented in the Methodology Registry and consistent with Wooldridge Section 5.4.
  • Both previous review findings are resolved: validation precedence is documented/tested, and notebook outputs are refreshed.
  • Parameter propagation, aggregation, serialization, and changelog coverage are complete.

Methodology

P3 — Documented library policy

  • Impact: None. "error" refuses an unsupported design without changing the estimand or fitting unidentified periods. "drop" preserves the existing Section 5.4 reduction.
  • Evidence: diff_diff/wooldridge.py:L1457-L1540, docs/methodology/REGISTRY.md:L2426-L2427, docs/methodology/papers/wooldridge-2025-review.md:L461-L477.
  • Concrete fix: None required.

No weighting, variance, SE, identification-predicate, or inference changes were introduced.

Code Quality

No findings. Constructor validation, transactional set_params(), fit-time revalidation, and failure-state preservation are coherent.

Performance

No findings. The new branch adds only constant-cost validation and metadata assignment.

Maintainability

No findings. The parameter reaches get_params(), all estimator methods, result objects, aggregation, and pickle migration.

Tech Debt

No untracked debt. The completed opt-out backlog row was removed and ledger entry M-147 records the policy.

Security

No findings or apparent secrets.

Documentation/Tests

No findings.

  • Validation precedence is now accurately documented and covered at tests/test_wooldridge.py:L4492-L4547.
  • Stored notebook summaries include the new policy line.
  • The changelog fragment follows the per-PR convention.
  • Independent execution was unavailable because the environment lacks pytest, NumPy, and pandas; changed Python files passed syntax parsing.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Sep 7, 2026
@igerber
igerber force-pushed the codex/eval-api-a9439f07 branch from f7afa23 to 4c4a178 Compare September 7, 2026 19:10
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 4c4a178eeccdb140e6b6494b2cdb9120db382408


Overall assessment

Looks good — no unmitigated P0 or P1 findings.

Executive summary

  • Prior validation-precedence and stale-notebook findings are resolved.
  • The new option affects Wooldridge ETWFE across OLS, logit, and Poisson without changing estimates, weighting, or inference.
  • The opt-out is documented in the Methodology Registry and preserves Wooldridge Section 5.4 under the default.
  • Parameter propagation, serialization, tests, and changelog coverage are complete.

Methodology

P3 — Documented library policy

  • Impact: None. "error" refuses unsupported designs; it never estimates unidentified periods. Default "drop" retains the Section 5.4 reduction.
  • Evidence: diff_diff/wooldridge.py:L1457-L1540, docs/methodology/REGISTRY.md:L2464-L2465, docs/methodology/papers/wooldridge-2025-review.md:L461-L477.
  • Concrete fix: None required.

No variance, SE, inference, weighting, or support-predicate changes were introduced.

Code Quality

No findings. Validation is eager, transactional through set_params(), and repeated at fit time.

Performance

No findings. The opt-out adds only validation and a linear observation-count calculation on the refusal path.

Maintainability

No findings. The parameter reaches constructor introspection, all estimator methods, result provenance, aggregation, and legacy-pickle migration.

Tech Debt

No untracked debt. The completed TODO item was removed and policy row M-147 records the behavior.

Security

No findings or apparent secrets.

Documentation/Tests

No findings.

  • Validation precedence is documented and tested at tests/test_wooldridge.py:L4492-L4547.
  • Stored notebook summaries were refreshed.
  • The 87-case matrix covers methods, control groups, rank policies, anticipation, survey designs, bootstrap, trends, state, and serialization.
  • A valid changelog.d/ fragment is present.
  • Runtime tests could not be independently executed because NumPy is unavailable; changed Python files passed syntax parsing.

@igerber
igerber merged commit f55bc06 into main Sep 7, 2026
31 checks passed
@igerber
igerber deleted the codex/eval-api-a9439f07 branch September 7, 2026 20:58
@igerber igerber mentioned this pull request Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant