4.0 migration guide + R-equivalents argument table (phase 4) - #759
Conversation
Ships the two un-rowed obligations from the 3.9-cut checklist (docs/v4-design.md section 9, items 2 and 3) - the last thing between 3.9 being code-complete and the formal cut. Lands BEFORE the release commit, the order the design already mandates in two normative places (the phase table at :828 and the hand-assertion sentence at :885); the release PR verifies these rather than shipping them. docs/migration-4.0.md is a 21-row orientation table plus a 108-row per-symbol appendix, worked before/after for the three merges, a codemod regex table, and a separate "already shipped in 3.9" section. The appendix row set is derived from the ledger by _changes_at_4_0 and asserted in both directions, so a rescheduled or added row fails CI until the guide is updated. THE APPENDIX HAS NO DERIVED ADVICE COLUMNS, and that is the load-bearing decision. An `Available since` column was designed, gated, and then removed: every label it would carry rests on a single ledger field while the fact asserted is compound. `introduced_in` tracks the dataclass STORAGE flip, so the nine field-flip rows say 4.0 while `.att` resolves today; a null `new` covers removals, default flips AND translations; a successor that resolves can still raise (the five bootstrapped-fit aggregate() families). No better probe fixes that, so the appendix carries only what is mechanically checkable and the compound claim moved into a hand-written Fix cell sourced from each row's notes. That choice was validated during implementation, not assumed. The generator's null-successor guard refused to emit a mechanical rename for M-087, then M-011/M-012/M-014/M-016 - each needed text written from its notes. Executing the robust= family produced THREE distinct rules across four rows: drop-or-translate on DifferenceInDifferences/LinearRegression, drop-only on TripleDifference (it accepts only hc1, so vcov_type="classical" RAISES), and the inverse on HAD, whose legacy default is non-robust. One pre-written rule would have been wrong on all four. Local review then found three defects that prove the same point one level down, in the hand-written prose rather than the derived table: - The auto-cluster opt-out was INVERTED. `cluster=None` and omission are the same thing under the 4.0 policy - both auto-cluster at unit - so the documented "reproduce 3.x exactly" recipe would have silently returned clustered SEs, p-values and CIs. `cluster=False` disables (v4-design.md:681). Also scoped: the TWFE event-study mode has auto-clustered since 3.9, so the flip changes the static and other panel paths, not that one. - M-139 pointed at a method that does not exist. `HADPretestReport.aggregate` is a metadata field; the successor is panel-shape inference. The advice raised. - All three merge examples carried an invalid keyword: `treated=` for `treatment=`, `partition=` for `eligibility=` on the RETIRED staggered class (the surviving TripleDifference does take `partition=`, which is what made it look right), and `treated=`/`post=` for `treatment=`/`time=` on both CiC sides. The third is the failure mode this page is structurally exposed to - it is markdown, and test_doc_snippets.py discovers only RST, so a plausible-but-wrong keyword ships silently. Rather than leave that acceptance bare, test_migration_guide_examples_bind_to_real_signatures ast-parses the guide's python blocks and asserts every constructor/fit() keyword exists on the target signature. It binds examples to reality WITHOUT executing 4.0-only behaviour, which is why the snippet harness could not do this job. Verified non-vacuous rather than assumed: replayed against the four original broken keywords it flags all four, and it checks 33 keywords on the live guide. The R argument table is scoped to the three packages r_comparison.rst actually evidences with paired R/Python blocks - did, HonestDiD, synthdid. fixest, DIDmultiplegtDYN and DIDHAD appear there in prose only, so they are named as out of scope rather than mapped from memory. Non-1:1 cases get a Notes cell instead of a fabricated row: aggte(type="dynamic") -> aggregate(type="event_study") (AGGREGATION_VOCABULARY has no "dynamic"), synthdid's block structure -> treatment indicator plus post_periods, and HonestDiD's betahat/sigma carried by the results object. Its gate is two-directional and resolves each name against the SPECIFIC callable the row names - resolving against any public callable would let `time` or `unit` be satisfied by an unrelated signature. The R side itself is ungateable (no R signatures in the repo) and the docstring says so. The gate earned itself immediately by catching a real defect in the table it guards: a cell read `aggregate(type=)` where the parameter is `type`. Two deviations from the approved plan, both forced by execution. The doc-deps union needed a private-module filter the plan did not specify - the mechanical recipe resolved `_reporting_helpers.py` to a key that does not exist; excluding private modules loses no coverage (both public owners are already in the union) and avoids a schema edit. And the no-op Fix denylist had to become word-boundary regex: a bare "none" fired on the legitimate `cluster=None` and `summary(alpha=None)` that two cells must state. TODO.md's aggregate= row is re-graded Quick -> Heavy with a measured inventory: 28 executable code-cell sites across 9 notebooks (14_continuous_did and 15_efficient_did match only in markdown prose; 21_had_pretest_workflow uses a spaced `aggregate =` that a naive grep misses), plus six docs files of which two API pages cannot migrate at all today because their report consumers read the raw event_study_effects field post-fit aggregate() never populates. Also corrects two false statements on the R page that no snippet gate can reach: `.ci` (the field is conf_int) and the claim that aggregation is requested at fit time. The llms.txt and README.md catalog entries are deliberately NOT here: both list pages as readthedocs.io/en/stable/..., and `stable` builds the newest tag (v3.8.0), so an entry added before the cut is a 404 in the wheel-bundled agent contract. They go with the release commit. Cross-references to TODO.md/DEFERRED.md/v4-design.md/the ledger YAML are inline literals, never links - none is a Sphinx document, so a MyST link to any of them fails the -W build. Verified: 469 gate tests pass (matrix incl. 8 new guide gates + 3 parser vacuity tests, docs IA incl. the new R-table gate, doc-deps integrity); naming guard clean (the guide is outside every scan lane); doc snippets 119 passed / 5 skipped with r_comparison block indices unshifted; Sphinx -W build succeeded with zero warnings; black, ruff, and mypy clean at the CI-pinned toolchain.
Overall assessmentExecutive summary
MethodologyP1 — SyntheticDiD migration can change estimates
Code QualityP2 — Example-signature gate is vacuous
PerformanceNo findings. MaintainabilityNo findings. Tech DebtP3 — Tracked documentation limitations
SecurityNo findings. Documentation/TestsP2 — Codemod table is incomplete
P2 — Results-field migration omits inference siblings
Runtime tests could not run because this checkout lacks Path to Approval
|
…arry their values Addresses the CI review on #759 (1 P1 + 3 P2, all verified against source before acting). P1 - THE SYNTHETICDID ADVICE WOULD HAVE CHANGED ESTIMATES. The appendix told readers to drop `lambda_reg`/`zeta` and "set `zeta_omega=`/`zeta_lambda=` instead". But those params have been IGNORED since 3.0.0 - `synthetic_did.py:216` warns "deprecated and ignored. Regularization is now auto-computed from data. Use zeta_omega to OVERRIDE". So 3.x behaviour is auto-computed regularization, and copying the old value into the new parameter activates an override that was previously inert, changing unit weights, ATT and inference. The value-preserving migration is to delete the argument and leave the new ones unset. Both cells now say that explicitly and warn against the mechanical copy. The ledger's own "use zeta_omega" note describes where the CAPABILITY moved, not a value migration - reading it as the latter is what produced the error. P2 - the results-field section documented only `overall_att` -> `att`, but the whole inference quintet moves (`overall_se`, `overall_t_stat`, `overall_p_value`, `overall_conf_int`). Added the mapping table, plus the ContinuousDiD outlier that M-058's notes call out: its siblings carry an `att` infix (`overall_att_se`, `overall_att_t_stat`, ...), so anyone grepping `overall_se` misses them entirely. P2 - the codemod table claimed to cover identifier renames but omitted `treatment_col` -> `takeup` (the one `*_col` param whose replacement is not just the suffix dropped), `aggregation` -> `level`, and `controls` -> `covariates`. Added, and the three renames that CANNOT be global replacements are now enumerated with the reason each is contextual: `group=` -> `unit=` would corrupt `TripleDifference.fit`, which takes a `group=` that is not renamed; `time=` -> `post=` survives everywhere else as the calendar column; `aggregate=` is not a rename at all. P2 - the signature-binding gate added in the previous round was itself vacuous: deleting every example, or renaming the ```python fence, emptied its loop and it passed. It now asserts a floor of 4 blocks and 20 resolvable keywords, and names the six estimators whose worked examples must be present. Verified by mutation rather than assumed - renaming the fence and deleting the QDiD example each fire the expected assertion, and the guide is restored afterwards. The gate I wrote to catch documentation errors had the same defect I built three parser-vacuity tests to prevent on the table parsers. Adding a floor to a new gate is not optional just because the gate is small. Verified: 469 gate tests pass; Sphinx -W build succeeded with zero warnings; black and ruff clean.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0/P1 findings. Executive summary
MethodologyNo findings. The corrected SyntheticDiD advice at Code QualityNo findings. PerformanceNo findings. MaintainabilityNo findings. Tech DebtP3 — Tracked aggregation limitations
SecurityNo findings. Documentation/TestsP3 — Markdown examples are not executed
Static ledger, parser, documentation-dependency, and syntax checks passed. Full pytest/Sphinx execution was unavailable because the environment lacks pytest, NumPy, pandas, SciPy, and Sphinx. |
Summary
Ships the two un-rowed obligations from the 3.9-cut checklist (
docs/v4-design.md§9, items 2 and 3) — the last thing between 3.9 being code-complete and the formal cut.docs/migration-4.0.md(new): a 21-row orientation table, a 108-row per-symbol appendix carrying the ledger's ownOld/Newlocators plus a one-line fix each, worked before/after for the three merges, a codemod regex table, and a separate "already shipped in 3.9" section.docs/r_comparison.rst: the rule-8 Argument mapping table — the library's stated alternative to shipping R-spelling parameter aliases.tests/test_v4_matrix.pyandtests/test_docs_ia.py, including three vacuity guards.Sequencing: this lands before the 3.9.0 release commit, which is the order the design already mandates in two normative places (the phase table at
:828and the hand-assertion sentence at:885). The release PR then verifies items 2 and 3 rather than shipping them. No ordering amendment was needed.The appendix has no derived advice columns — the load-bearing decision
An
Available sincecolumn was designed, gated, and then removed. Every label it would carry rests on a single ledger field while the fact asserted is compound:introduced_infield-fliprows say4.0while.attresolves todaynew is nullaggregate-postfitsuccessors raise on bootstrapped fitsNo better probe fixes that, so the appendix carries only what is mechanically checkable and the compound claim moved into a hand-written
Fixcell sourced from each row's notes.This was validated during implementation, not assumed. The generator's null-successor guard refused to emit a mechanical rename five times (
M-087, thenM-011/M-012/M-014/M-016). Executing therobust=family produced three distinct rules across four rows — drop-or-translate onDifferenceInDifferences/LinearRegression, drop-only onTripleDifference(it accepts onlyhc1, sovcov_type="classical"raises), and the inverse on HAD. One pre-written rule would have been wrong on all four.What local review caught
Three defects, all in hand-written prose rather than the derived table — the same failure mode one level down:
cluster=Noneand omission are the same thing under the 4.0 policy;cluster=Falsedisables (v4-design.md:681). The documented "reproduce 3.x exactly" recipe would have silently returned clustered SEs, p-values and CIs.M-139pointed at a method that does not exist.HADPretestReport.aggregateis a metadata field; the successor is panel-shape inference.treated=/partition=/post=fortreatment=/eligibility=/time=. Note the survivingTripleDifferencereally does takepartition=, which is what made the mistake look right.That third one is the risk this page is structurally exposed to (markdown isn't snippet-executed), so rather than leave the acceptance bare,
test_migration_guide_examples_bind_to_real_signaturesAST-parses the guide'spythonblocks and asserts every constructor/fit()keyword exists on the target signature — binding examples to reality without executing 4.0-only behaviour. Verified non-vacuous: replayed against the four original broken keywords it flags all four, and it checks 33 keywords on the live guide.Deliberately not here
The
llms.txtandREADME.mdcatalog entries. Both list pages asreadthedocs.io/en/stable/…, andstablebuilds the newest tag (v3.8.0), so an entry added before the cut is a 404 in the wheel-bundled agent contract. They go with the release commit.Methodology references (required if estimator / math changes)
diff_diff/source files changed. This is documentation plus test gates; no estimator, equation, weighting, variance, or inference code is touched.docs/methodology/REGISTRY.md(pooled-vs-within MPD/TWFE, the staggered DDD merge) and the Athey–Imbens 2006 review (the CiC-over-QDiD recommendation).docs/v4-design.md§10 carries a dated amendment recording five deviations from its own skeleton (two tables not one; no availability column, with the reason; the §7b and §9 additions; the codemod's identifier-only scope).Validation
tests/test_v4_matrix.py(8 migration-guide gates + 3 parser/vacuity tests),tests/test_docs_ia.py(the two-directional R-table gate).test_doc_snippets.py119 passed / 5 skipped withr_comparisonblock indices unshifted;SPHINXOPTS="-W" make -C docs htmlreports build succeeded with zero warnings; black, ruff and mypy clean at the CI-pinned toolchain.Security / privacy