Skip to content

docs: migrate tutorial notebooks off fit-time aggregate= (sweep PR-B) - #764

Merged
igerber merged 2 commits into
mainfrom
docs/aggregate-postfit-notebooks
Aug 11, 2026
Merged

docs: migrate tutorial notebooks off fit-time aggregate= (sweep PR-B)#764
igerber merged 2 commits into
mainfrom
docs/aggregate-postfit-notebooks

Conversation

@igerber

@igerber igerber commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Completes the fit-time aggregate= teaching sweep (notebook half; follows docs: migrate narrative RST off fit-time aggregate= (sweep PR-A) #763): 16 analytical fit/configuration sites across 02_staggered_did, 16_survey_did (including its 200-draw Monte-Carlo loop), 26_composition_drift_calibration (native seam + the two balance-adapter sites, where the kwarg was not load-bearing), 16_wooldridge_etwfe, 17_brand_awareness_survey, and 24_staggered_vs_collapsed_power now fit plain and aggregate post-fit via results.aggregate(type=...).
  • Legacy event_study_effects/group_effects dict reads are rebound to the containers with keyed lookups that preserve each notebook's missing-event-time fallbacks (the compared grids differ in size, so positional pairing would misalign rows); cells that printed the fit-time summary() aggregation blocks print the container summaries instead, so no rendered teaching content is lost; plot_event_study and compute_honest_did consume the containers directly.
  • Deliberate keeps, each annotated in-notebook: the BOOTSTRAPPED fits (02 cell 20, both 09_real_world_examples case studies) retain fit-time aggregate= with comments naming it the documented until-4.0 exception (post-fit recompute levels raise on a bootstrapped fit; 'simple' relays), with the expected FutureWarning visible in the refreshed outputs and acknowledged in adjacent markdown; 08_triple_diff's staggered DDD keeps fit-time as the CANONICAL route (rows M-140/M-141 - no post-fit successor exists on its container).
  • tests/test_t26_composition_drift_calibration_drift.py mirrors the tutorial-26 migration at all three of its sites (the fit_survey_cs mirror, the legacy-field read, and the adapter-parity call).
  • TODO.md: the sweep row is retired; the kept teaching surfaces stay deviation-grep-resolvable by moving onto the staggered-DDD post-fit-aggregate() row and the diagnostic_report-admission row Locations.
  • All 8 edited notebooks were re-executed with outputs written back, under DIFF_DIFF_BACKEND=python (the backend both notebook CI jobs and RTD-rendered outputs use) on this worktree's kernel. Diff-size note: the ~6,700 insertions are dominated by refreshed outputs plus nbformat's canonical normalization of two notebooks previously stored as compact single-string-source JSON; the reviewable source delta is the cell-source changes described above.

Methodology references (required if estimator / math changes)

  • Method name(s): N/A - documentation/tutorial-only; no estimator or math changes. Post-fit aggregation reuses the same aggregation machinery the fit-time path called (Callaway & Sant'Anna 2021 group-time -> aggregated parameters, mirroring R did's post-estimation aggte()).
  • Paper / source link(s): N/A
  • Any intentional deviations from the source (and why): None

Validation

  • Tests added/updated: tests/test_t26_composition_drift_calibration_drift.py (three mirror sites migrated; one assertion now reads the post-fit container). Locally: pytest --nbmake over all 8 edited notebooks passes (8/8, DIFF_DIFF_BACKEND=python); tests/test_t24_staggered_vs_collapsed_power_drift.py (incl. slow-marked) and tests/test_t26_* pass; a scoped sweep (grep -rn "aggregate=" docs/*.rst docs/api/*.rst docs/tutorials/*.ipynb) confirms the only remaining fit-time sites are the sanctioned inventory (bootstrap exceptions, canonical DDD, blocked report API pages, already-correct prose).
  • Backtest / simulation / notebook evidence (if applicable): refreshed committed outputs for all 8 notebooks; migrated cells render warning-free, kept-exception cells render the expected FutureWarning.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Sixteen analytical fit/configuration sites across 02_staggered_did,
16_survey_did (incl. the 200-draw Monte-Carlo loop),
26_composition_drift_calibration (native seam + the two balance-adapter
sites where the kwarg was not load-bearing), 16_wooldridge_etwfe,
17_brand_awareness_survey and 24_staggered_vs_collapsed_power now fit
plain and aggregate post-fit, with legacy event_study_effects/group_effects
dict reads rebound to the containers via keyed lookups that preserve each
notebook's missing-event-time fallbacks; cells that printed the fit-time
summary() aggregation blocks print the container summaries instead. The
BOOTSTRAPPED fits (02 cell 20, both 09 case studies) keep fit-time
aggregate= with comments naming it the documented until-4.0 exception, and
08_triple_diff's staggered DDD keeps it as the canonical route
(M-140/M-141).

tests/test_t26_composition_drift_calibration_drift.py mirrors the t26
migration at all three sites. TODO.md retires the sweep row; the kept
teaching surfaces move onto the DDD post-fit-aggregate() and
diagnostic_report-admission rows' Locations so the deviation-grep still
resolves them. All edited notebooks re-executed under
DIFF_DIFF_BACKEND=python on the diffdiff-dev kernel; compact-JSON
notebooks were normalized to nbformat's canonical layout by the
write-back.
@github-actions

Copy link
Copy Markdown

Overall assessment: ✅ Looks good

Executive summary

  • No unmitigated P0/P1 findings.
  • No estimator, weighting, variance, or identification logic changed.
  • Post-fit aggregation preserves event-time alignment and survey inference.
  • Bootstrap and staggered-DDD exceptions are documented and tracked.
  • Independent tests could not run because this environment lacks pytest/NumPy.

Methodology

  • Severity: P3 informational
    Impact: None. Changes use the existing Callaway–Sant’Anna aggregation machinery and preserve HonestDiD covariance/provenance (docs/tutorials/17_brand_awareness_survey.ipynb:L1058).
    Concrete fix: None required.

Code Quality

  • Severity: None
    Impact: Keyed lookups correctly avoid mismatching differing event-time grids.
    Concrete fix: None.

Performance

  • Severity: None
    Impact: The Monte Carlo loop still performs one event-study aggregation per fit; no material regression identified.
    Concrete fix: None.

Maintainability

  • Severity: None
    Impact: Tutorials now consistently use public post-fit containers.
    Concrete fix: None.

Tech Debt

  • Severity: P3 informational
    Impact: Bootstrap re-aggregation and staggered-DDD post-fit aggregation remain known limitations, tracked in TODO.md:L24 and TODO.md:L54.
    Concrete fix: None required in this PR.

Security

  • Severity: P3
    Impact: Refreshed outputs contain machine-specific absolute paths and a local username, but no credentials or secrets (docs/tutorials/17_brand_awareness_survey.ipynb:L433).
    Concrete fix: Optionally sanitize warning filenames or execute notebooks from a neutral workspace path.

Documentation/Tests

  • Severity: None
    Impact: All changed notebooks contain execution counts and no stored error outputs; the t26 mirror test uses the new container (tests/test_t26_composition_drift_calibration_drift.py:L320).
    Concrete fix: None.

…eview P3)

The re-execution wrote warning lines carrying the local checkout's absolute
path (username included) and ipykernel temp paths into the committed
outputs of five notebooks; the pre-PR outputs carried no machine paths.
Sanitizes outputs only - library warning paths become repo-relative
(diff_diff/survey.py:NNNN:) and kernel temp paths become <ipython-cell>.py;
the deliberately-rendered FutureWarning text is unchanged. Sources
untouched.
@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 75c8b459b7e6dbf11016d4f3afefcb158deff638


Overall assessment: ✅ Looks good

Executive summary

  • No unmitigated P0/P1 findings.
  • No estimator, weighting, variance, or identification logic changed.
  • Post-fit containers preserve keyed event-time alignment and HonestDiD covariance.
  • Bootstrap and staggered-DDD exceptions match documented contracts and TODO.md.
  • The prior machine-path P3 is resolved; no secrets found.
  • Tests could not run because NumPy/pytest are unavailable; all notebooks have execution counts and no stored errors.

Methodology

  • Severity: P3 informational
    Impact: Migrations use the existing Callaway–Sant’Anna post-fit aggregation contract, including survey inference and HonestDiD consumption (docs/tutorials/17_brand_awareness_survey.ipynb:L1058, L1198).
    Concrete fix: None.

Code Quality

  • Severity: None
    Impact: Keyed lookups correctly handle unequal event-time grids (docs/tutorials/02_staggered_did.ipynb:L1494, L2102).
    Concrete fix: None.

Performance

  • Severity: None
    Impact: No material computation regression identified.
    Concrete fix: None.

Maintainability

  • Severity: None
    Impact: Analytical tutorials now consistently use public post-fit containers.
    Concrete fix: None.

Tech Debt

  • Severity: P3 informational
    Impact: Bootstrap re-aggregation and staggered-DDD aggregation remain explicitly tracked (TODO.md:L24, L54).
    Concrete fix: None required here.

Security

  • Severity: P3 informational
    Impact: The previous machine-specific path disclosure is resolved; refreshed warnings use <ipython-cell>.py.
    Concrete fix: None.

Documentation/Tests

  • Severity: None
    Impact: The t26 mirror test uses post-fit aggregation (tests/test_t26_composition_drift_calibration_drift.py:L320); notebook metadata contains no execution errors.
    Concrete fix: None.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Aug 11, 2026
@igerber
igerber merged commit 90ad96d into main Aug 11, 2026
39 of 40 checks passed
@igerber
igerber deleted the docs/aggregate-postfit-notebooks branch August 11, 2026 12:34
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