From 8f217dd41a1fefd609f30f6a38bcd736d8043636 Mon Sep 17 00:00:00 2001 From: igerber Date: Mon, 10 Aug 2026 18:15:04 -0400 Subject: [PATCH 1/5] docs: teach post-fit results.aggregate() in narrative RST (sweep PR-A) Migrates the narrative-docs half of the fit-time aggregate= teaching sweep (TODO row, M-020 family) onto post-fit results.aggregate(type=): choosing_estimator.rst drops its second full refit, python_comparison.rst now mirrors the differences package's post-fit idiom, r_comparison.rst maps 1:1 onto R's three aggte() calls, and troubleshooting.rst's event-study-plot remedy plots the aggregate('event_study') container with an explicit reference_period. The troubleshooting bootstrap passage deliberately keeps fit-time aggregate= with a comment naming it the documented until-4.0 exception (post-fit recompute levels raise on a bootstrapped fit; 'simple' relays). The two rewritten blocks are now self-contained and removed from _CONTEXT_DEPENDENT_SNIPPETS so the snippet harness actually executes them. --- CHANGELOG.md | 18 ++++++++++++++++++ docs/choosing_estimator.rst | 8 +++----- docs/python_comparison.rst | 5 ++--- docs/r_comparison.rst | 13 +++++++------ docs/troubleshooting.rst | 24 ++++++++++++++---------- tests/test_doc_snippets.py | 2 -- 6 files changed, 44 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7d15dc2..fac113ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed +- **Narrative docs migrated off the deprecated fit-time `aggregate=`** (the + 3.9 M-020 family; TODO "fit-time aggregate= teachings" sweep, RST half): + `choosing_estimator.rst`, `python_comparison.rst` and `r_comparison.rst` now + teach post-fit `results.aggregate(type=...)` (the r_comparison block maps + 1:1 onto R's three `aggte()` calls; `'all'` has no post-fit counterpart), + and `troubleshooting.rst`'s event-study-plot remedy is rewritten to + `plot_event_study(results.aggregate('event_study'), reference_period=-1)`. + The troubleshooting bootstrap passage deliberately KEEPS fit-time + `aggregate='event_study'` with a comment naming it the documented until-4.0 + exception (post-fit recompute levels raise on a bootstrapped fit; + `'simple'` relays). The two rewritten blocks that previously died on + allowlisted `NameError`s (`r_comparison:block2`, `troubleshooting:block8`) + are now self-contained and removed from `_CONTEXT_DEPENDENT_SNIPPETS`, so + `tests/test_doc_snippets.py` actually executes them. + ## [3.9.0] - 2026-08-10 The 3.9 shim release of the 4.0 API-unification program (`docs/v4-design.md`): diff --git a/docs/choosing_estimator.rst b/docs/choosing_estimator.rst index 6bdf40f6..7ab24c00 100644 --- a/docs/choosing_estimator.rst +++ b/docs/choosing_estimator.rst @@ -246,11 +246,9 @@ This is the recommended estimator for most applied work with staggered adoption. # Overall ATT print(f"Overall ATT: {results.overall_att:.3f}") - # Event study aggregation - es = cs.fit(data, outcome='y', unit='unit_id', - time='period', first_treat='first_treat', - covariates=['x1', 'x2'], aggregate='event_study') - event_study_df = es.to_dataframe('event_study') + # Event study aggregation (post-fit - no refit needed) + es = results.aggregate('event_study') + event_study_df = es.to_dataframe() Reversible (Non-Absorbing) Treatment ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/python_comparison.rst b/docs/python_comparison.rst index 05609aa8..5ae13420 100644 --- a/docs/python_comparison.rst +++ b/docs/python_comparison.rst @@ -412,10 +412,9 @@ Staggered DiD (Callaway-Sant'Anna) unit='unit', time='time', first_treat='first_treat', - covariates=['x1', 'x2'], - aggregate='event_study' + covariates=['x1', 'x2'] ) - event_study = results.event_study_effects + event_study = results.aggregate('event_study') .. code-block:: python diff --git a/docs/r_comparison.rst b/docs/r_comparison.rst index 5cd4b3c7..fc1eba78 100644 --- a/docs/r_comparison.rst +++ b/docs/r_comparison.rst @@ -204,13 +204,14 @@ staggered DiD. Here's how to translate common operations: .. code-block:: python - # Python (R's aggte() has two counterparts: the fit-time aggregate= shown here, - # deprecated in 3.9, and post-fit results.aggregate(type=), which supersedes it) + # Python (post-fit results.aggregate(type=) is aggte()'s counterpart; + # R's "dynamic" is spelled "event_study") + cs = CallawaySantAnna() results = cs.fit(data, outcome='Y', time='period', unit='id', - first_treat='G', aggregate='all') - overall_att = results.overall_att # Simple aggregation - event_study = results.event_study_effects # Dynamic - by_group = results.group_effects # By cohort + first_treat='G') + agg_simple = results.aggregate('simple') + agg_dynamic = results.aggregate('event_study') + agg_group = results.aggregate('group') R ``HonestDiD`` Package → diff-diff ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index 4db61373..baee9c36 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -209,7 +209,11 @@ Staggered Adoption Issues # Check cohort sizes print(data.groupby('first_treat')['unit_id'].nunique()) - # Use bootstrap for better inference + # Use bootstrap for better inference. On a BOOTSTRAPPED fit, post-fit + # results.aggregate('event_study') raises - the percentile draws are not + # retained - so the deprecated fit-time aggregate= remains the documented + # route for this case until 4.0. (results.aggregate('simple') relays the + # stored bootstrap inference and works on any fit.) cs = CallawaySantAnna(n_bootstrap=999) results = cs.fit(data, outcome='y', unit='unit_id', time='period', first_treat='first_treat', @@ -233,17 +237,17 @@ Visualization Issues from diff_diff import plot_event_study - # Check your results first - print(results.period_effects) # or results.event_study_effects + # For CallawaySantAnna, aggregate to an event study post-fit, then plot + cs = CallawaySantAnna() + results = cs.fit(data, outcome='y', unit='unit_id', + time='period', first_treat='first_treat') + es = results.aggregate('event_study') - # Specify reference period explicitly - plot_event_study(results, reference_period=-1) + # Check the surface first + print(es.to_dataframe()) - # For CallawaySantAnna, fit with aggregate='event_study' - results = cs.fit(data, outcome='y', unit='unit_id', - time='period', first_treat='first_treat', - aggregate='event_study') - plot_event_study(results) + # Specify reference period explicitly + plot_event_study(es, reference_period=-1) "Plot doesn't show in Jupyter" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/test_doc_snippets.py b/tests/test_doc_snippets.py index 4e41ce6e..e3fed6ab 100644 --- a/tests/test_doc_snippets.py +++ b/tests/test_doc_snippets.py @@ -413,11 +413,9 @@ def _restore_datasets_module(): "python_comparison:block5", "quickstart:block3", "quickstart:block9", - "r_comparison:block2", "r_comparison:block3", "r_comparison:block4", "r_comparison:block7", - "troubleshooting:block8", } From dbf9d42a9e90b39ec589398497aaab64debb06ee Mon Sep 17 00:00:00 2001 From: igerber Date: Mon, 10 Aug 2026 18:22:00 -0400 Subject: [PATCH 2/5] docs: troubleshooting plot example uses base_period='universal' (review R1) Verified by execution: the varying-base default's event-study container has no reference row (is_reference all False; e=-1 is an estimated pre-treatment effect), so plot_event_study(es, reference_period=-1) would renormalize around an estimate - the REGISTRY varying-base common-reference warning. The universal-base fit marks e=-1 as the true reference, keeping the section's specify-the-reference-explicitly lesson coherent. --- CHANGELOG.md | 8 ++++++-- docs/troubleshooting.rst | 9 ++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fac113ea..d3d54f49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `choosing_estimator.rst`, `python_comparison.rst` and `r_comparison.rst` now teach post-fit `results.aggregate(type=...)` (the r_comparison block maps 1:1 onto R's three `aggte()` calls; `'all'` has no post-fit counterpart), - and `troubleshooting.rst`'s event-study-plot remedy is rewritten to - `plot_event_study(results.aggregate('event_study'), reference_period=-1)`. + and `troubleshooting.rst`'s event-study-plot remedy is rewritten to a + `base_period="universal"` fit plotted via + `plot_event_study(results.aggregate('event_study'), reference_period=-1)` - + universal, not the varying default, because under a varying base `e=-1` is + an estimated effect and explicit renormalization around it would shift + every plotted point (REGISTRY's varying-base common-reference warning). The troubleshooting bootstrap passage deliberately KEEPS fit-time `aggregate='event_study'` with a comment naming it the documented until-4.0 exception (post-fit recompute levels raise on a bootstrapped fit; diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index baee9c36..81e71d1b 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -237,13 +237,16 @@ Visualization Issues from diff_diff import plot_event_study - # For CallawaySantAnna, aggregate to an event study post-fit, then plot - cs = CallawaySantAnna() + # For CallawaySantAnna, aggregate to an event study post-fit, then plot. + # Use base_period="universal" so e=-1 is a true common reference - under + # the default varying base, e=-1 is an ESTIMATED pre-treatment effect and + # normalizing the plot around it would silently shift every point. + cs = CallawaySantAnna(base_period='universal') results = cs.fit(data, outcome='y', unit='unit_id', time='period', first_treat='first_treat') es = results.aggregate('event_study') - # Check the surface first + # Check the surface first (the is_reference column marks e=-1) print(es.to_dataframe()) # Specify reference period explicitly From f2485eb13bb9ae5988d0286b33086c8963329bc8 Mon Sep 17 00:00:00 2001 From: igerber Date: Mon, 10 Aug 2026 18:26:24 -0400 Subject: [PATCH 3/5] docs: plot the container's own reference instead of hard-coding -1 (review R2) On gapped period grids the universal-base positional reference can sit at an event time other than -1 (the reference_event_times multi-anchor case the common-reference guards exist for), so the example now inspects the container's is_reference row(s) and plots without a manual override. --- CHANGELOG.md | 11 ++++++----- docs/troubleshooting.rst | 17 ++++++++++------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3d54f49..255ae85b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 teach post-fit `results.aggregate(type=...)` (the r_comparison block maps 1:1 onto R's three `aggte()` calls; `'all'` has no post-fit counterpart), and `troubleshooting.rst`'s event-study-plot remedy is rewritten to a - `base_period="universal"` fit plotted via - `plot_event_study(results.aggregate('event_study'), reference_period=-1)` - - universal, not the varying default, because under a varying base `e=-1` is - an estimated effect and explicit renormalization around it would shift - every plotted point (REGISTRY's varying-base common-reference warning). + `base_period="universal"` fit whose `aggregate('event_study')` container is + plotted directly - the container carries its own `is_reference` marking, so + the example inspects the actual reference row(s) rather than hard-coding + `reference_period=-1` (under the varying default every pre-period point is + an estimated effect with no common anchor, and on gapped grids even the + universal base can anchor at an event time other than -1). The troubleshooting bootstrap passage deliberately KEEPS fit-time `aggregate='event_study'` with a comment naming it the documented until-4.0 exception (post-fit recompute levels raise on a bootstrapped fit; diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index 81e71d1b..ff8149e3 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -238,19 +238,22 @@ Visualization Issues from diff_diff import plot_event_study # For CallawaySantAnna, aggregate to an event study post-fit, then plot. - # Use base_period="universal" so e=-1 is a true common reference - under - # the default varying base, e=-1 is an ESTIMATED pre-treatment effect and - # normalizing the plot around it would silently shift every point. + # base_period="universal" gives the event study explicit reference row(s), + # marked in the container - under the default varying base every + # pre-treatment point is an estimated effect with no common anchor, so + # renormalizing a plot around one would silently shift every point. cs = CallawaySantAnna(base_period='universal') results = cs.fit(data, outcome='y', unit='unit_id', time='period', first_treat='first_treat') es = results.aggregate('event_study') - # Check the surface first (the is_reference column marks e=-1) - print(es.to_dataframe()) + # Inspect the actual reference row(s) - on gapped period grids the + # positional base can sit at an event time other than -1 + print(es.to_dataframe().query("is_reference")) - # Specify reference period explicitly - plot_event_study(es, reference_period=-1) + # Plot the container - it carries its own reference; no manual + # reference_period override is needed (or safe to hard-code) + plot_event_study(es) "Plot doesn't show in Jupyter" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 4498f15917ee588470631be186f64aec4afec0eb Mon Sep 17 00:00:00 2001 From: igerber Date: Mon, 10 Aug 2026 18:37:32 -0400 Subject: [PATCH 4/5] docs: import CallawaySantAnna in the self-contained troubleshooting block (CI review) The block constructs CallawaySantAnna but only imported plot_event_study - a copy-paste into a fresh session would NameError; the snippet harness masked it by preloading every public name. --- docs/troubleshooting.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index ff8149e3..4745622f 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -235,7 +235,7 @@ Visualization Issues .. code-block:: python - from diff_diff import plot_event_study + from diff_diff import CallawaySantAnna, plot_event_study # For CallawaySantAnna, aggregate to an event study post-fit, then plot. # base_period="universal" gives the event study explicit reference row(s), From eab34b30ec580571df9bf12d2fda09997a1c6878 Mon Sep 17 00:00:00 2001 From: igerber Date: Mon, 10 Aug 2026 19:57:21 -0400 Subject: [PATCH 5/5] docs: trim the sweep TODO row to its remaining notebook scope (review P3) The narrative-RST half this PR completes no longer reads as outstanding; the row now carries only the notebook half and the blocked API pages. --- TODO.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index c79db7b1..67fbbbf3 100644 --- a/TODO.md +++ b/TODO.md @@ -78,7 +78,7 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m |-------|----------|--------|--------|----------| | Committed `fixest::feols` event-study golden for TWFE `event_study=True` (within + pooled specs, unbalanced + covariate panels, matched CR1 cluster convention, per-period effects + vcov block) - the in-suite gates are shared-core cross-checks (TWFE-within == MPD-absorb, pooled == MPD bit-exact), so a defect common to the shared core would pass; the live-R harness (`benchmarks/R/benchmark_multiperiod.R`, `feols(y ~ treated * time_f \| unit)`) validated the within design in `docs/benchmarks.rst` but is not a committed regression test - follow the `fixest_did_twfe_golden.json` committed-golden pattern (pytest.skip when absent) | `tests/test_fixest_did_twfe_parity.py`, `benchmarks/R/` | 3(a) R2 | Mid | Medium | | Type-blind `n_bootstrap` acceptance in already-validated estimators - HAD bool (`isinstance(..., int)` passes `True`, runs as 1 replicate), dCDH bool+float (its bare `< 0` check passes both `True` and `2.5`), TROP float (`2.5` passes the `>= 2` floor), SyntheticDiD float under all three variance methods + bool/negative under jackknife (its floor check is skipped there) - align these local checks with the `utils.validate_n_bootstrap` type guard (M-081 kept them out of the sweep: it scoped to previously-UNvalidated estimators only) | `diff_diff/had.py`, `diff_diff/chaisemartin_dhaultfoeuille.py`, `diff_diff/trop.py`, `diff_diff/synthetic_did.py` | 2(d) PR-B | Quick | Low | -| Fit-time `aggregate=` teachings persist across the docs and tutorials (M-020 family, removed at 4.0); migrate to post-fit `results.aggregate(...)`. Re-scoped 2026-08-09 while shipping the migration guide - this is NOT a quick sweep. Narrative docs: `troubleshooting.rst:216`/`:245` both sit on the `n_bootstrap=999` fit at `:213`, and post-fit event-study aggregation **raises `NotImplementedError` on a bootstrapped fit** (`staggered_results.py:323`), so those two need a decision about what to teach before any edit; `choosing_estimator.rst:252` and `python_comparison.rst:416` use the analytical default and can migrate freely; `r_comparison.rst:119-127` needs its `results.event_study_effects`/`.group_effects` reads rebound to the `aggregate()` return values in the same edit (the fields stay `None` after post-fit aggregation). API pages: `docs/api/triple_diff.rst:56`, plus `business_report.rst:77` and `diagnostic_report.rst:59`, which **cannot** migrate today because both report consumers read the raw `event_study_effects` field; `had.rst:164` and `continuous_did.rst:137` are prose references only. Tutorials: **28 executable code-cell sites across 9 notebooks** (`02_staggered_did` 7, `09_real_world_examples` 6, `16_survey_did` 6, `26_composition_drift_calibration` 3, `21_had_pretest_workflow` 2, and one each in `08_triple_diff`, `16_wooldridge_etwfe`, `17_brand_awareness_survey`, `24_staggered_vs_collapsed_power`) - all nbmake-executed; `14_continuous_did` and `15_efficient_did` match only in markdown prose | `docs/troubleshooting.rst`, `docs/choosing_estimator.rst`, `docs/python_comparison.rst`, `docs/r_comparison.rst`, `docs/api/*.rst`, `docs/tutorials/*.ipynb` | 2(b) PR-4 | Heavy | Medium | +| Fit-time `aggregate=` teachings persist in the tutorials (M-020 family, removed at 4.0); migrate to post-fit `results.aggregate(...)`. The NARRATIVE-RST half is DONE (this row's original scope, re-scoped 2026-08-09): `choosing_estimator.rst`, `python_comparison.rst`, `r_comparison.rst` migrated; `troubleshooting.rst`'s bootstrap passage deliberately keeps fit-time `aggregate=` as the documented until-4.0 exception (post-fit recompute levels **raise `NotImplementedError` on a bootstrapped fit**, `staggered_results.py:323`). Remaining: **the notebook half** - executable code-cell sites across 9 notebooks (`02_staggered_did` 7, `09_real_world_examples` 6, `16_survey_did` 6, `26_composition_drift_calibration` 3, `21_had_pretest_workflow` 2, and one each in `08_triple_diff`, `16_wooldridge_etwfe`, `17_brand_awareness_survey`, `24_staggered_vs_collapsed_power`) - all nbmake-executed; `14_continuous_did`/`15_efficient_did` match only in markdown prose; bootstrapped fits keep the documented fit-time exception and 08's staggered DDD is canonical (M-140/M-141). API pages stay per their blockers: `docs/api/triple_diff.rst:56` (canonical DDD), `business_report.rst:77`/`diagnostic_report.rst:59` (report consumers read the raw `event_study_effects` field); `had.rst:164`/`continuous_did.rst:137` are prose-only and already correct | `docs/tutorials/*.ipynb`, `docs/api/*.rst` | 2(b) PR-4 | Heavy | Medium | | Evaluate adding the `BaseEstimator` param surface (get_params/set_params) to the exported classes that never had it - `PowerAnalysis`, `LinearRegression`, `BusinessReport`, `DiagnosticReport`, `TWFEWeightsResult` (a NEW public surface, deliberately out of the 2(c)-i pure-refactor scope; `LinearRegression` is the one `fit`-bearing class excluded from the contract suite's roster-completeness test). | `diff_diff/linalg.py`, `diff_diff/power.py` | mixin PR | Mid | Low | | Tighten the mypy suppressions that back the enforced-zero posture: burn down `prep_dgp`'s per-module `[index]` override (needs a None-vs-array restructure that preserves the seeded RNG stream), and evaluate re-enabling the globally disabled codes (`arg-type`, `return-value`, `var-annotated`, `assignment`) one at a time — `assignment` alone hid several real annotation drifts found during the 2026-07 triage. | `pyproject.toml` `[tool.mypy]`, `diff_diff/prep_dgp.py` | lint-CI | Mid | Low | | MMM interop follow-up: Meridian `roi_calibration_period` mask builder - accept the MMM's time index + channel order and emit the boolean `(n_media_times, n_media_channels)` mask so `.to_code()` scopes the prior to the experiment window automatically (today the caller passes a mask expression / `full_model_window=True`). | `diff_diff/mmm.py` | mmm-interop | Quick | Low |