docs: migrate tutorial notebooks off fit-time aggregate= (sweep PR-B) - #764
Merged
Conversation
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.
Overall assessment: ✅ Looks goodExecutive summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
|
…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.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment: ✅ Looks goodExecutive summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
aggregate=teaching sweep (notebook half; follows docs: migrate narrative RST off fit-time aggregate= (sweep PR-A) #763): 16 analytical fit/configuration sites across02_staggered_did,16_survey_did(including its 200-draw Monte-Carlo loop),26_composition_drift_calibration(native seam + the twobalance-adapter sites, where the kwarg was not load-bearing),16_wooldridge_etwfe,17_brand_awareness_survey, and24_staggered_vs_collapsed_powernow fit plain and aggregate post-fit viaresults.aggregate(type=...).event_study_effects/group_effectsdict 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-timesummary()aggregation blocks print the container summaries instead, so no rendered teaching content is lost;plot_event_studyandcompute_honest_didconsume the containers directly.02cell 20, both09_real_world_examplescase studies) retain fit-timeaggregate=with comments naming it the documented until-4.0 exception (post-fit recompute levels raise on a bootstrapped fit;'simple'relays), with the expectedFutureWarningvisible 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.pymirrors the tutorial-26 migration at all three of its sites (thefit_survey_csmirror, the legacy-field read, and the adapter-parity call).aggregate()row and the diagnostic_report-admission row Locations.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)
did's post-estimationaggte()).Validation
tests/test_t26_composition_drift_calibration_drift.py(three mirror sites migrated; one assertion now reads the post-fit container). Locally:pytest --nbmakeover all 8 edited notebooks passes (8/8,DIFF_DIFF_BACKEND=python);tests/test_t24_staggered_vs_collapsed_power_drift.py(incl. slow-marked) andtests/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).FutureWarning.Security / privacy