Skip to content

feat(v4): PR 4c-ii-a of the 4.0 cut - remove MultiPeriodDiD, MultiPeriodDiDResults, PeriodEffect and EventStudy - #870

Merged
igerber merged 2 commits into
mainfrom
feat/v4-pr4c-ii-remove-mpd
Oct 26, 2026
Merged

igerber merged 2 commits into
mainfrom
feat/v4-pr4c-ii-remove-mpd

Conversation

@igerber

@igerber igerber commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • 4.0 cut, PR 4c-ii-a (docs/v4-design.md section 9 item 4c-ii). The MultiPeriodDiD family is removed: MultiPeriodDiD, MultiPeriodDiDResults, PeriodEffect and the EventStudy alias raise ImportError; results_base._from_mpd, estimators._mpd_results_from_payload, the shared core's post-average (avg_*) block and the payload fields only the wrapper read are gone. DifferenceInDifferences._fit_event_study_core now serves only TwoWayFixedEffects.fit(event_study=True) (post_periods required by the core; include_treatment_main / cluster_override keyword-required). Item 4c-ii is split in two because the single diff exceeded the AI review input budget: this PR carries everything a CI gate or the REGISTRY deviation rule needs in the same diff as the source; PR 4c-ii-b (stacked on this branch) carries the methodology/benchmark prose, the tracking-file rewordings, the new live benchmark arm and the oracle fixture's mpd_twin prune.
  • M-016: EventStudyResults.period_effects is a FutureWarning compatibility view (removed 5.0) - a private frozen record per non-reference row (period, effect, se, t_stat, p_value, conf_int, is_significant, significance_stars), keyed by calendar label, is_significant at the surface's alpha (user decision 2026-10-25; the removed record hard-coded 0.05), duplicate labels rejected, GATED to time_scale="calendar" (relative-time aggregate("event_study") surfaces raise AttributeError, so hasattr is False and nothing warns). build_event_study_surface opens with an isinstance(EventStudyResults) passthrough.
  • M-083: static TwoWayFixedEffects.fit(time=) raises ValueError naming post= (the TripleDifference M-085 pattern); the M-082 shim and twfe.py's resolve_renamed_kwarg call are gone. M-007: the "default reference_period has changed" warning is retired.
  • Kept on purpose (REGISTRY Note + TODO row, user decision 2026-10-25): the clustered hc2_bm contrast batch keeps the former post-period-average column as the anchor of the batch-relative Bell-McCaffrey DOF noise-floor guard - dropping it flips near-collinear per-period DOFs from NaN to finite (reproduced on 20/299 draws), so per-period NaN masks stay bit-identical; its own DOF is no longer read.
  • Consumer MPD branches deleted: honest_did / pretrends native branches, the visualization period_effects plot branches, DiagnosticReport / BusinessReport / _reporting_helpers / practitioner name-keyed branches (user decision: delete, not keep dormant; the EventStudyResults admission TODO row records where they lived), the power registry entry (the generic fallback reads the flat att/se/p_value/conf_int names and raises TypeError naming _EstimatorProfile otherwise). pyproject.toml filterwarnings entry removed.
  • Ledger: M-010/M-011/M-012 -> removed, M-060/M-007/M-083 -> done, M-016 -> shimmed on diff_diff:EventStudyResults.period_effects, M-082 keeps shimmed with warning: null (M-031 pattern), M-050 note corrected. Spec item 4c-ii carries the SHIPPED note incl. the three recorded deviations (anchor column; benchmark arm as a new live script because the refresh_2026_07 harness runs the frozen one in pinned 3.x venvs; DR/BR branch deletion). Migration guide: unit= auto-cluster caveat, the period_effects view, the M-016 appendix row. REGISTRY: archive note on the 4a template, the two TWFE event-study Notes, the absorb auto-route verification narrowed to the two live pins.

Methodology references (required if estimator / math changes)

  • Method name(s): N/A - no estimator or math changes. The TWFE event-study surface is pinned field for field against the committed pre-refactor capture (tests/fixtures/v4_twfe_event_study_oracles.json, 11 lanes at 68e383b plus two clustered hc2_bm lanes captured at d45d531 on the unmodified tree BEFORE the core pruning, because the CR2 bypass had no committed lane; rust agreed with python exactly).
  • Paper / source link(s): N/A
  • Any intentional deviations from the source (and why): None in the math. The retained post-average contrast column is documented (REGISTRY TWFE event-study Note; TODO row for the guard redesign).

Validation

  • Tests added/updated: tests/test_v4_merge_mpd.py rewritten for the removal (name-absence pins for the four names and the internal adapters, the core's own post_periods raise, the M-083 raise, the M-007 retired-warning guard, TestPeriodEffectsView incl. the relative-surface gate, datetime labels, duplicate labels, alpha, stars, the builder passthrough and the view-never-fires consumer sweep, and the structural anchor pin test_hc2_bm_contrast_batch_keeps_post_average_anchor); tests/test_twfe_event_study_decoupling.py TWFE-only (+ the sorted-contract pin for the calendar builder); MPD-only tests deleted or re-fixtured onto cs_fit / the TWFE surface across the estimator, vcov_type, honest/pretrends, survey, Conley, visualization, report, practitioner, power, alias, roster and inference-policy suites; the clustered per-period CR2 DOF R/clubSandwich pin and the event-study covariate-collision guard are MIGRATED to TWFE rather than deleted; power gains regression tests for the new fallback contract; variance-conventions twfe_es_* rows re-pinned (tail_df 5) and the committed table regenerated. Full default suite on the complete (a+b) tree: 15139 passed, 193 skipped; PR a's own gates: decoupling test 31 (python + rust), merge test 73, naming guard / matrix / tracking / variance / docs IA / snippets / guides / doc-deps / changelog / vcov_type / surface roster 1301 passed. CI-pinned mypy diff_diff: no issues; black/ruff clean; py3.9 compileall of the touched tests clean; scipy-1.13.1 smoke of the oracle test green.
  • Backtest / simulation / notebook evidence (if applicable): two markdown cells only (06_power_analysis estimator count 12 -> 11 and its registry table row; 16_survey_did survey-support row), no re-execution.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Changelog

  • changelog.d/ fragment added (or N/A - no user-visible change): Yes - changelog.d/20261025-v4-pr4c-ii-remove-mpd.md (### Removed, ### Breaking Changes, ### Internal)

Follow-ups

  • PR 4c-ii-b (stacked on this branch; base retargets to main once this lands): REGISTRY / REPORTING / survey-theory / METHODOLOGY_REVIEW prose, docs/benchmarks.rst, survey-roadmap, performance-plan, CONTRIBUTING, the repo debugging-notes file, the rust comment, TODO rows 33/35/38/95/99, three DEFERRED retirements, the new benchmarks/python/benchmark_twfe_event_study.py arm (R parity verified locally), and the oracle fixture's mpd_twin prune with the lane-metadata cleanup.
  • TODO rows added here: TWFE event-study power profile; batch-relative hc2_bm DOF guard redesign (with the reproduction recipe).
  • Cut sequence continues with item 5 (storage flip).

…iodDiDResults, PeriodEffect and EventStudy

Cut-sequence item 4c-ii (docs/v4-design.md section 9), split into 4c-ii-a
(this commit: everything a CI gate or the REGISTRY deviation rule needs in
the same diff as the source) and 4c-ii-b (methodology/benchmark prose,
tracking-file rewordings and the oracle fixture's mpd_twin prune) because the
single diff exceeded the local reviewer's input cap.

Source: the class, its container and per-period record, the EventStudy
alias, results_base._from_mpd, estimators._mpd_results_from_payload, the
core's post-average (avg_*) block and the payload fields only the wrapper
read are gone; the shared event-study core is TWFE-only (post_periods
required by the core, include_treatment_main/cluster_override
keyword-required). The hc2_bm contrast batch KEEPS the former post-average
column as the batch-relative Bell-McCaffrey DOF guard's anchor (dropping it
flips near-collinear per-period DOFs from NaN to finite; REGISTRY Note + TODO
row). M-016: period_effects is a FutureWarning property view on calendar
EventStudyResults (private frozen record, is_significant at the surface
alpha, duplicate labels rejected, AttributeError on relative-time surfaces);
build_event_study_surface opens with an isinstance passthrough. M-083:
static fit(time=) raises naming post=. M-007 warning retired. Consumer MPD
branches deleted (honest_did, pretrends, visualization, diagnostic_report,
business_report, _reporting_helpers, practitioner, power - the power generic
fallback reads the flat att/se/p_value/conf_int names). pyproject
filterwarnings entry removed.

Oracle: two clustered hc2_bm lanes captured at d45d531 BEFORE the pruning
(the CR2 bypass had no committed lane; rust agreed exactly); every TWFE lane
of tests/fixtures/v4_twfe_event_study_oracles.json passes unchanged.

Ledger: M-010/M-011/M-012 -> removed, M-060/M-007/M-083 -> done, M-016 ->
shimmed on EventStudyResults, M-082 shimmed with warning: null, M-050 note
corrected. Spec item 4c-ii SHIPPED note with the recorded deviations.
Migration guide: unit= caveat, period_effects view, appendix row. REGISTRY:
archive note, TWFE event-study Notes (alpha, anchor column), auto-route
verification narrowed to the live pins. API rst / autosummary stubs /
homepage table / README catalog / llms guides / user docs / two notebook
markdown cells / doc-deps drop the removed names.

Tests: MPD-only tests deleted or re-fixtured onto cs_fit / the TWFE surface;
the clustered per-period CR2 DOF R pin and the event-study covariate-collision
guard are migrated to TWFE; tests/test_v4_merge_mpd.py carries the removal,
M-016, M-007 and anchor pins; variance-conventions twfe_es rows re-pinned
(tail_df 5) and the table regenerated.
@github-actions

Copy link
Copy Markdown

Overall assessment

✅ Looks good — no unmitigated P0 or P1 findings identified.

Executive summary

  • Core pruning preserves the surviving TWFE event-study calculations and safe_inference() path.
  • All original 11 oracle lanes remain unchanged; two clustered hc2_bm lanes were added.
  • The compatibility view handles empty results, duplicates, reference rows, NaNs, and significance at the surface’s alpha.
  • Remaining capability gaps are documented and tracked.
  • Static checks passed; dependency-based tests could not run locally.

Methodology

Affected methods: pooled/within TWFE event studies, Bell–McCaffrey/CR2 inference, and HonestDiD/PreTrendsPower extraction.

Registry equations, in-code references, and local Roth/Rambachan–Roth source audits support the surviving contracts. No new weighting, covariance, degrees-of-freedom, control-group, or identification changes were found. External papers were not independently retrieved.

  • P3 — Informational: retained contrast anchor.
    Location: diff_diff/estimators.py:L2137-L2209; docs/methodology/REGISTRY.md:L478-L490.
    Impact: Preserves the existing batch-relative guard and per-period NaN masks. This is an explicitly documented implementation choice.
    Concrete fix: None required.

Code Quality

No findings. Changed core callers supply the required keywords, and per-period inference continues through safe_inference().

Performance

No findings. Numerical kernels are unchanged; unused post-average computation and payload fields are removed.

Maintainability

No findings. Surviving runtime code contains no references to the removed classes or adapters. Surface passthrough prevents accidental compatibility-property warnings.

Tech Debt

  • P3 — Informational: successor workflow gaps remain tracked.
    Location: TODO.md:L33; TODO.md:L89-L90.
    Impact: TWFE event-study power simulation lacks a dedicated profile, and report admission remains unavailable.
    Concrete fix: No change required for this PR; implement the tracked power profile, survey builder, and source-aware report routing in follow-ups.

Security

No findings. No introduced secrets or unsafe execution paths identified.

Documentation/Tests

No findings requiring changes. The changelog fragment follows repository conventions, and migrated tests retain the R/clubSandwich per-period inference check.

Validation performed:

  • All 56 changed Python files compiled and parsed for Python 3.9.
  • Changelog validation passed.
  • Isolated execution of the actual compatibility getter passed its edge-case checks.

Focused pytest execution was unavailable because the environment lacks pytest, NumPy, pandas, and SciPy. Numerical regression tests were inspected but not executed.

…p behind an optional matplotlib

CI's test legs do not install matplotlib (an optional dependency), so the new
view-never-fires sweep in tests/test_v4_merge_mpd.py failed on every leg at
`import matplotlib`. The non-plot consumers (to_dataframe/to_dict/summary,
build_event_study_surface, compute_honest_did, compute_pretrends_power)
always run; plot_event_study / plot_honest_event_study run only when
matplotlib imports, as the visualization suite already does.
@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: ed1a9639bbf8691a1ac39fe20c5955e0ec1bf54d


Overall assessment

✅ Looks good — re-review confirms no unmitigated P0 or P1 findings.

Executive summary

  • Surviving TWFE estimation and per-period safe_inference() calculations remain unchanged.
  • All 11 original oracle records are unchanged; two clustered hc2_bm lanes were added.
  • The compatibility view preserves stored inference and handles duplicates, empty rows, reference omission, and surface alpha.
  • The latest test adjustment keeps non-plotting consumer checks active without matplotlib.
  • Static validation passed; numerical tests could not run because dependencies are unavailable.

Methodology

Affected methods: pooled/within TWFE event studies, Bell–McCaffrey/CR2 inference, and HonestDiD/pre-trends extraction.

Cross-checked against the Methodology Registry, cited in-code formulas, and repository paper audits for Roth (2022) and Rambachan–Roth (2023). Original papers were not independently retrieved. No undocumented methodological change was identified. Static comparison confirms surviving consumer extraction logic is unchanged after legacy-branch removal.

  • P3 — Informational: retained contrast anchor.
    Location: diff_diff/estimators.py:L2137-L2209; docs/methodology/REGISTRY.md:L480-L490.
    Impact: Retaining the average contrast preserves the existing batch-relative guard and per-period NaN masks. This behavior is explicitly documented.
    Concrete fix: None required.

Code Quality

No findings. Required core keywords are supplied by the surviving caller. No new inline inference calculations or executable references to removed names were found. The power fallback’s targeted TypeError propagates through simulation error handling.

Performance

No findings. Removed average-result calculations reduce unnecessary work; numerical kernels remain unchanged.

Maintainability

No findings. Surface passthrough avoids triggering the deprecated property, and surviving consumers no longer inspect period_effects.

Tech Debt

  • P3 — Informational: successor capabilities remain tracked.
    Location: TODO.md:L33; TODO.md:L89-L91.
    Impact: Report admission, a dedicated TWFE event-study power profile, additional R parity coverage, and guard redesign remain follow-up work. These limitations are explicitly recorded.
    Concrete fix: None required for this PR; implement the recorded follow-ups separately.

Security

No findings. No introduced secrets or unsafe execution paths were identified.

Documentation/Tests

No findings requiring changes. The changelog fragment covers the behavior changes and passes repository validation. The migrated R/clubSandwich test retains per-period degrees-of-freedom and confidence-interval checks.

Validation performed:

  • All 56 changed Python files compiled and parsed for Python 3.9.
  • Changelog validation passed.
  • Original oracle records were verified unchanged.
  • Isolated execution of the actual compatibility getter passed edge-case checks using standard-library stand-ins.

Full pytest and numerical regression execution were unavailable because pytest, NumPy, pandas, and SciPy are absent.

@igerber
igerber merged commit 2eeff79 into main Oct 26, 2026
42 checks passed
@igerber
igerber deleted the feat/v4-pr4c-ii-remove-mpd branch October 26, 2026 11:35
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