Repository navigation
feat(v4): PR 4c-ii-a of the 4.0 cut - remove MultiPeriodDiD, MultiPeriodDiDResults, PeriodEffect and EventStudy - #870
Conversation
…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.
Overall assessment✅ Looks good — no unmitigated P0 or P1 findings identified. Executive summary
MethodologyAffected 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.
Code QualityNo findings. Changed core callers supply the required keywords, and per-period inference continues through PerformanceNo findings. Numerical kernels are unchanged; unused post-average computation and payload fields are removed. MaintainabilityNo findings. Surviving runtime code contains no references to the removed classes or adapters. Surface passthrough prevents accidental compatibility-property warnings. Tech Debt
SecurityNo findings. No introduced secrets or unsafe execution paths identified. Documentation/TestsNo findings requiring changes. The changelog fragment follows repository conventions, and migrated tests retain the R/clubSandwich per-period inference check. Validation performed:
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.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — re-review confirms no unmitigated P0 or P1 findings. Executive summary
MethodologyAffected 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.
Code QualityNo 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 PerformanceNo findings. Removed average-result calculations reduce unnecessary work; numerical kernels remain unchanged. MaintainabilityNo findings. Surface passthrough avoids triggering the deprecated property, and surviving consumers no longer inspect Tech Debt
SecurityNo findings. No introduced secrets or unsafe execution paths were identified. Documentation/TestsNo 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:
Full pytest and numerical regression execution were unavailable because pytest, NumPy, pandas, and SciPy are absent. |
Summary
docs/v4-design.mdsection 9 item 4c-ii). The MultiPeriodDiD family is removed:MultiPeriodDiD,MultiPeriodDiDResults,PeriodEffectand theEventStudyalias raiseImportError;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_corenow serves onlyTwoWayFixedEffects.fit(event_study=True)(post_periodsrequired by the core;include_treatment_main/cluster_overridekeyword-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'smpd_twinprune.EventStudyResults.period_effectsis aFutureWarningcompatibility 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_significantat the surface'salpha(user decision 2026-10-25; the removed record hard-coded 0.05), duplicate labels rejected, GATED totime_scale="calendar"(relative-timeaggregate("event_study")surfaces raiseAttributeError, sohasattris False and nothing warns).build_event_study_surfaceopens with anisinstance(EventStudyResults)passthrough.TwoWayFixedEffects.fit(time=)raisesValueErrornamingpost=(the TripleDifference M-085 pattern); the M-082 shim andtwfe.py'sresolve_renamed_kwargcall are gone. M-007: the "default reference_period has changed" warning is retired.hc2_bmcontrast 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.period_effectsplot branches, DiagnosticReport / BusinessReport /_reporting_helpers/practitionername-keyed branches (user decision: delete, not keep dormant; theEventStudyResultsadmission TODO row records where they lived), the power registry entry (the generic fallback reads the flatatt/se/p_value/conf_intnames and raisesTypeErrornaming_EstimatorProfileotherwise).pyproject.tomlfilterwarnings entry removed.M-010/M-011/M-012->removed,M-060/M-007/M-083->done,M-016->shimmedondiff_diff:EventStudyResults.period_effects,M-082keepsshimmedwithwarning: null(M-031 pattern),M-050note corrected. Spec item 4c-ii carries the SHIPPED note incl. the three recorded deviations (anchor column; benchmark arm as a new live script because therefresh_2026_07harness runs the frozen one in pinned 3.x venvs; DR/BR branch deletion). Migration guide:unit=auto-cluster caveat, theperiod_effectsview, 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)
tests/fixtures/v4_twfe_event_study_oracles.json, 11 lanes at 68e383b plus two clusteredhc2_bmlanes captured at d45d531 on the unmodified tree BEFORE the core pruning, because the CR2 bypass had no committed lane; rust agreed with python exactly).Validation
tests/test_v4_merge_mpd.pyrewritten for the removal (name-absence pins for the four names and the internal adapters, the core's ownpost_periodsraise, the M-083 raise, the M-007 retired-warning guard,TestPeriodEffectsViewincl. the relative-surface gate, datetime labels, duplicate labels, alpha, stars, the builder passthrough and the view-never-fires consumer sweep, and the structural anchor pintest_hc2_bm_contrast_batch_keeps_post_average_anchor);tests/test_twfe_event_study_decoupling.pyTWFE-only (+ the sorted-contract pin for the calendar builder); MPD-only tests deleted or re-fixtured ontocs_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-conventionstwfe_es_*rows re-pinned (tail_df5) 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-pinnedmypy diff_diff: no issues; black/ruff clean; py3.9compileallof the touched tests clean; scipy-1.13.1 smoke of the oracle test green.06_power_analysisestimator count 12 -> 11 and its registry table row;16_survey_didsurvey-support row), no re-execution.Security / privacy
Changelog
changelog.d/20261025-v4-pr4c-ii-remove-mpd.md(### Removed,### Breaking Changes,### Internal)Follow-ups
mainonce 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 newbenchmarks/python/benchmark_twfe_event_study.pyarm (R parity verified locally), and the oracle fixture'smpd_twinprune with the lane-metadata cleanup.hc2_bmDOF guard redesign (with the reproduction recipe).