From 5abc2a2158da8d42e1f41fb8f65e33d89de8e749 Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 12:36:07 -0400 Subject: [PATCH 1/8] fix: summary-alpha guard sweep (M-146), zero-SE plot gate, lambda-slope dedup, shared validate_pscore_trim (M-145) --- CHANGELOG.md | 57 ++++++++ DEFERRED.md | 1 + TODO.md | 5 +- diff_diff/_dr_scores.py | 44 +++++- .../chaisemartin_dhaultfoeuille_results.py | 12 +- diff_diff/continuous_did.py | 16 ++- diff_diff/dml_did.py | 23 +-- diff_diff/efficient_did_results.py | 11 +- diff_diff/imputation_results.py | 11 +- diff_diff/lwdid.py | 10 +- diff_diff/results_base.py | 20 +++ diff_diff/stacked_did_results.py | 11 +- diff_diff/staggered.py | 10 +- diff_diff/staggered_results.py | 11 +- diff_diff/staggered_triple_diff_results.py | 11 +- diff_diff/sun_abraham.py | 11 +- diff_diff/triple_diff.py | 26 +--- diff_diff/two_stage_results.py | 11 +- diff_diff/utils.py | 25 ++++ diff_diff/visualization/_event_study.py | 88 ++++++++++-- docs/methodology/REGISTRY.md | 33 ++++- docs/migration-4.0.md | 19 ++- docs/v4-deprecations.yaml | 28 +++- docs/v4-design.md | 7 +- tests/test_chaisemartin_dhaultfoeuille.py | 26 ++++ tests/test_continuous_did.py | 28 ++++ tests/test_dr_scores.py | 21 +++ tests/test_efficient_did.py | 25 ++++ tests/test_event_study_consumers.py | 131 ++++++++++++++++++ tests/test_imputation.py | 26 ++++ tests/test_lwdid.py | 8 ++ tests/test_stacked_did.py | 33 +++++ tests/test_staggered.py | 82 +++++++++++ tests/test_staggered_triple_diff.py | 25 ++++ tests/test_sun_abraham.py | 26 ++++ tests/test_two_stage.py | 26 ++++ tests/test_v4_matrix.py | 24 ++-- tests/test_v4_merge_ddd.py | 4 + tests/test_visualization_new.py | 76 ++++++++++ tests/test_visualization_plotly.py | 30 ++++ 40 files changed, 973 insertions(+), 119 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0033bb570..018af339d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,63 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 committed `DoubleMLDIDCSBinary` characterization spike (no parity oracle exists — DoubleML's RCS score differs and omits the λ term). +### Changed +- **`ContinuousDiD` rejects `pscore_trim=0`** (ledger row M-145): the bound + tightens from `[0, 0.5)` to `(0, 0.5)` — `trim=0` disabled the + `np.clip(pscore, trim, 1 - trim)` overlap guard that keeps the `1/(1-p)` + IPW/DR weights finite (the TripleDifference M-142 rationale). Validation now + runs through the shared `utils.validate_pscore_trim`, so the error message + wording changed, non-real-scalar inputs (`None`, strings, `Decimal`/ + `Fraction`, 1-element arrays) raise `ValueError` instead of `TypeError` (or + silent acceptance, for the 1-element array), and the stored value is coerced + to built-in `float`. +- **`pscore_trim` validation unified on `utils.validate_pscore_trim`** + (M-145): `CallawaySantAnna` gains the same type guard (previously a bare + range check: `None`/str raised `TypeError`, a 1-element array and + `Decimal`/`Fraction` were accepted) at construction and at the fit-path + mutation re-check; `TripleDifference`, `CallawaySantAnna`, and + `ContinuousDiD` now store the value coerced to built-in `float`; `LWDiD`'s + message wording changed (guard behavior unchanged). The deprecated + `StaggeredTripleDifference` deliberately keeps its permissive construction + shape (M-013/M-144 posture). +- **Staggered-family `summary(alpha=...)`/`print_summary(alpha=...)` reject a + non-fit alpha** (M-146): `CallawaySantAnnaResults` and its siblings + (StaggeredTripleDiff, ChaisemartinDHaultfoeuille, Imputation, EfficientDiD, + TwoStage, Stacked, SunAbraham results) previously relabeled the + confidence-interval header at the requested alpha while printing the + fit-time stored intervals — silent coverage mislabeling (bootstrap + percentile intervals cannot be reconstructed from the SE). A value different + from the fit-time `alpha` now raises `ValueError` via the shared + `results_base._require_fit_alpha` guard (the DMLDiD/EventStudyResults + precedent); `alpha=0.0`, previously swallowed by a falsy-`or` default, + raises too. Re-fit at the desired alpha instead. + +### Fixed +- **`plot_event_study` zero-SE pointwise gate** (the `plot_group_effects` + twin): the `effect ± z·SE` reconstruction NaN-gates zero/negative-SE rows — + their stored inference is all-NaN, and a finite zero-width interval + presented defined inference for them. Auto-inferred reference rows (effect + 0, se 0) on the raw `event_study_effects` route retain their degenerate + constraint bar per the REGISTRY reference-retention contract; the + `EventStudyResults` container route after an explicit `reference_period=` + (which discards stored-interval overrides) no longer draws spurious + zero-width bars either. +- **`plot_honest_event_study` raw (non-container) routes** now mirror the + container's retained-row semantics: zero/non-finite-SE periods are excluded + up front instead of drawing a zero-width original CI beside honest + inference that was never computed for them (explicitly requesting one + raises); the reference period is auto-inferred (a `reference_period` + attribute or HonestDiD's own constraint-row signature — never a bare `-1` + fallback), enabling the existing reference tolerance on these routes; an + all-undefined surface raises instead of rendering a blank figure. + +### Internal +- `DMLDiD`'s repeated-cross-section cell loop computes the Case 2 λ-slope + `Ĝ₂λ` once per cell via `_dr_scores._chang_rcs_score_augmented_with_slope` + (previously twice: inside `chang_rcs_score_augmented` and again for the + `g2_lambda` diagnostic, each with its own input-validation pass). Public + score functions and all numerics unchanged (bitwise-pinned). + ## [3.10.0] - 2026-08-22 ### Added diff --git a/DEFERRED.md b/DEFERRED.md index f0840b8ee..25c394b72 100644 --- a/DEFERRED.md +++ b/DEFERRED.md @@ -139,6 +139,7 @@ decisions (refactor waivers, perf trade-offs, test-infrastructure calls) are rec | Decision | Location | Verified | |----------|----------|----------| +| **`utils.validate_pscore_trim` ships without the `allow_zero` flag the retired TODO row proposed.** The row (retired in the M-145 PR) suggested `validate_pscore_trim(value, *, allow_zero)`; after ContinuousDiD's alignment to `0 < x < 0.5`, no consumer needs `allow_zero=True` — every migrated copy (TripleDifference, DMLDiD, ContinuousDiD, LWDiD, CallawaySantAnna) wants the strict interval, and the deliberately-unmigrated `StaggeredTripleDifference` keeps its own bare check rather than a flagged call. A dead-on-arrival parameter on a shared validator is a drift magnet; re-add the flag only when a real caller needs a `[0, 0.5)` domain, with a distinct range message | `diff_diff/utils.py` | M-145 / 2026-08-27 | | **Fixed-B Stata SE-golden comparisons in the LWDiD suite run unmarked in default CI** (no `ci_params.bootstrap()` scaling, no `slow` marker): the SE tolerance derives from BOTH fixed rep counts (ours B=999, Stata's R from the golden meta) so scaling either side would invalidate the committed-golden comparison — the ci_params convention governs convergence-style tests, not fixed-B golden comparisons — and these tests are the PR #588 acceptance bar, which must run in default CI; module-scoped fit memoization bounds the cost (~1-2 s per fit locally) | `tests/test_methodology_lwdid.py` | 2026-08-16 | | **The 4.0 migration guide's code blocks are not snippet-executed.** `tests/test_doc_snippets.py` discovers a hardcoded list of `.rst` files and only `.. code-block:: python` / RST `::` bodies, so `docs/migration-4.0.md` gets no coverage. Deliberate: the guide is a MIXED document - most "after" examples (the renames, `results.att`) run on the current release, but the `field-flip` and `df-convention-flip` examples describe 4.0 behaviour that cannot run until 4.0, so a blanket execution lane would fail by construction. Closing the gap fully means a markdown-fence extractor plus a skip-marker convention for the future-API blocks - a harness change, out of scope for a docs PR. What IS gated: the appendix's ledger parity (`test_migration_guide_*`), which pins the row set and every mechanically checkable cell; and, since the first local review found all three merge examples carrying invalid keywords, `test_migration_guide_examples_bind_to_real_signatures`, which ast-parses the guide's python blocks and asserts every constructor/`fit()` keyword exists on the target signature. That is signature binding, NOT execution - it deliberately skips calls whose owner it cannot resolve (e.g. `results.aggregate(...)`), and it cannot catch a wrong VALUE or a wrong sequence of calls. The hand-written `Fix` prose remains unverifiable by any available means. | `docs/migration-4.0.md`, `tests/test_doc_snippets.py` | Phase 4 / 2026-08-09 | | **MMM launch carousel carries scoped marketing claims, not exporter-contract documentation.** The deck (a LinkedIn marketing artifact, not a docs surface) states capabilities in scoped-but-punchy form: guardrail copy says "the easy mistakes fail loudly" / "you own the design, it owns the math" (the caller-owned estimand/population/window/outcome-scale alignment lives in REGISTRY.md's MMM section and the exporter docstrings, not on slides), and slide 8 shows tutorial 29's worked staggered-boost lift row WITHOUT an on-slide linearity qualifier - the compression's linear-channel scoping is the tutorial's job, which the CTA points to. Absolutes ("no silent mis-calibration", "any estimate exports", "anything with an estimate + SE") are ban-listed by `tests/test_mmm_carousel_claims.py`; local review R1/R2 pressed for compliance-style scoping language on the slides and the marketing-appropriate scoped copy was chosen instead. | `carousel/generate_mmm_carousel.py`, `tests/test_mmm_carousel_claims.py` | mmm-carousel / 2026-08-20 | diff --git a/TODO.md b/TODO.md index 5c78c9a3c..69764c3f5 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,7 @@ Related tracking surfaces: | Issue | Location | Origin | Effort | Priority | |-------|----------|--------|--------|----------| +| Non-staggered `summary(alpha=...)` mislabel audit (the M-146 staggered-family guard's remaining siblings): `results.py:199` and `continuous_did_results.py:250` are VERIFIED mislabels (requested-alpha `{conf_level}%` header over stored fit-time `conf_int` — dual-review round 4); `results.py:792/:1290`, `synthetic_control_results.py:499`, `triple_diff.py:163`, `trop_results.py:207` carry the same `alpha = alpha or self.alpha` idiom and still need verification (a genuine recomputation from the SE at the requested alpha is legitimate on analytical fits). Apply the shared `results_base._require_fit_alpha` guard (or a real recomputation) per site. Include `plot_dose_response`'s raw-DataFrame CI reconstruction (`diff_diff/visualization/_continuous.py:105-108`): zero-SE rows draw a zero-width band — a weaker instance (user-supplied `se` column, no stored inference to contradict) of the M-146-adjacent zero-SE gate | `diff_diff/results.py`, `diff_diff/continuous_did_results.py`, `diff_diff/synthetic_control_results.py`, `diff_diff/triple_diff.py`, `diff_diff/trop_results.py`, `diff_diff/visualization/_continuous.py` | #794 follow-ups review | Mid | Medium | | Consolidate the remaining estimator-entangled DR/logit score variants (`staggered.py::_doubly_robust` + RC twins, `triple_diff.py`, `lwdid.py`, `wooldridge.py`) onto the shared `_dr_scores.py` module, each migration with its own committed oracle capture (the ContinuousDiD lift's two-tier pattern in `tests/test_dr_scores.py`); and add a ridge vcov path to `solve_ridge` if an estimator ever needs analytical ridge inference | `diff_diff/_dr_scores.py` | dml-b0 | Mid | Low | | hc2/hc2_bm floor `1 - h_ii` at 1e-10 in the shared leverage meat, fabricating finite (if inflated) variances for leverage-one observations - hc3 now fails closed there (LWDiD fix wave) but the pre-existing hc2 family behavior is released surface; decide fail-closed vs keep-floor for hc2/hc2_bm | `diff_diff/linalg.py` | #588 | Quick | Low | | Numeric between-period cohorts (e.g. `first_treat=4.5` with integer times) are rejected by LWDiD while CallawaySantAnna estimates them and LWDiD's own datetime/Period cohorts map to the next observed period — close the dtype asymmetry by adopting the next-observed-period mapping for numeric cohorts too (contract documented in REGISTRY cohort-encodings Note + `docs/api/lwdid.rst` Input Contract). Lands only after PR #588 merges | `diff_diff/lwdid.py` | #588 | Quick | Low | @@ -29,7 +30,6 @@ Related tracking surfaces: | Post-fit `aggregate()` for the staggered DDD container: `StaggeredTripleDiffResults` carries no `AggregationMixin`, which is why the phase-3(b) merge had to carry fit-time `aggregate=`/`balance_e=` onto the surviving `TripleDifference` (rows M-140/M-141) as the ONE documented exception to the section-6 aggregate-postfit program. Porting the container onto the M-122 aggregation contract retires both rows; note the bootstrapped-fit recompute levels will need replay or a fail-closed relay — solved for CS, DMLDiD, and EfficientDiD via the BootstrapReplaySpec state replay (the container port can adopt the same mechanism); ImputationDiD/TwoStageDiD/ContinuousDiD still track theirs. Until it lands, the DDD docs deliberately keep teaching the fit-time kwarg (the canonical route there) | `diff_diff/staggered_triple_diff_results.py`, `diff_diff/aggregation.py`, `docs/api/triple_diff.rst`, `docs/tutorials/08_triple_diff.ipynb` | 3(b) | Heavy | Medium | | Staggered-DDD power support: `simulate_power`/`simulate_mde`/`simulate_sample_size` now REJECT a staggered-configured `TripleDifference` (both registered DDD generators emit 2x2x2 data and fit with `(group, partition, post)`, so a staggered config would be simulated under the wrong design). Support needs a staggered DDD DGP profile plus fit-kwargs builder, and a decision on whether the mode is selected by profile or by the estimator's own config | `diff_diff/power.py` | 3(b) | Mid | Low | | Bootstrap-`seed` provenance on multiplier-bootstrap results containers: neither `StaggeredTripleDiffResults` nor `CallawaySantAnnaResults` carries the `seed` that generated its bootstrap SEs / p-values / sup-t bands, so a serialized result cannot report the random configuration behind its inference. NOT a 3(b) regression - `seed` reaches the engine and `get_params()` correctly (same seed reproduces the SE bit-exactly, a different seed moves it), the gap is results-object observability only, it predates the merge, and both containers inherit it from the shared `CallawaySantAnnaBootstrapMixin`. Add `seed` (and consider `n_bootstrap`/`bootstrap_weights`/`cband`) to BOTH containers plus `to_dict()`, with seeded and unseeded pins; sequence it with the M-014 container unification rather than schema-changing one container mid-merge. Precedent for exposing it: `ContinuousDiDResults`, `EfficientDiDResults`, `SyntheticDiDResults` already do | `diff_diff/staggered_triple_diff_results.py`, `diff_diff/staggered_results.py` | 3(b) | Quick | Low | -| `ContinuousDiD.pscore_trim` still validates `0.0 <= x < 0.5`, i.e. it admits `0`, while `TripleDifference` tightened to `0 < x < 0.5` in phase 3(b) (row M-142) on the grounds that `trim=0` disables the `np.clip(pscore, trim, 1-trim)` overlap guard keeping the `1/(1-p)` weights finite. The same argument applies to ContinuousDiD; aligning it was out of scope for a DDD merge and is recorded in the REGISTRY staggered-mode Note rather than left as silent drift. `TripleDifference` additionally gained a TYPE guard in 3(b) (reject bool/non-real-scalar/non-finite BEFORE the range comparison) because a bare `0 < x < 0.5` raises an incidental `TypeError` on `None`/str/complex/list, an ambiguous-truth error on a multi-element array, and silently ACCEPTS a 1-element array as the parameter; `ContinuousDiD`'s `np.isfinite(self.pscore_trim) and ...` has the same hole. Aligning both is one change - promote the guard to a shared `utils.validate_pscore_trim(value, *, allow_zero)` alongside `validate_n_bootstrap` rather than copying it (DMLDiD added a third inline copy of the TripleDifference guard in its shipping PR — fold it into the promotion) | `diff_diff/continuous_did.py`, `diff_diff/dml_did.py`, `diff_diff/utils.py` | 3(b) | Quick | Low | | Staggered-mode cluster-robust ANALYTICAL SEs: `cluster=` raises in `TripleDifference`'s staggered mode (and is accepted-then-ignored on the deprecated class), so clustered inference there is bootstrap-only. Implementing a clustered analytical path for the GMM-combined influence function would let the raise become a real lane | `diff_diff/_staggered_triple_diff_engine.py` | 3(b) | Heavy | Low | | diagnostic_report admission for `EventStudyResults` surfaces (the TWFE event-study mode + `aggregate('event_study')` containers): DiagnosticReport/BusinessReport now REJECT the surface explicitly (Phase 3(a); previously a silent zero-check report / all-null headline) and practitioner_next_steps serves the generic fall-through - admission needs source-aware routing (the type-name-keyed `_APPLICABILITY`/`_HANDLERS` registries cannot discriminate the unified container's producers) and a scalar-vs-per-period headline design; MPD-native results received {parallel_trends, pretrends_power, sensitivity, bacon, design_effect} | `diff_diff/diagnostic_report.py`, `diff_diff/business_report.py`, `diff_diff/practitioner.py` | 3(a) | Mid | Medium | | DiagnosticReport public skip bookkeeping omits RUNNER-level skips: `applicable_checks` reflects only gate outcomes, so a check whose runner returns `status="skipped"` (heterogeneity's empty/non-finite branches, `_pt_event_study`'s empty-coefs branch - a pre-existing pattern, now also reachable via a failed post-fit event-study derivation on bootstrapped / kit-less ImputationDiD/TwoStageDiD/ContinuousDiD fits) stays listed as applicable while `skipped_checks`/`schema["skipped"]` omit it, so automation reading the public fields can misclassify the check as completed; reconcile runner-returned skipped sections into the public bookkeeping (all checks, one convention) or resolve those availabilities at the gate | `diff_diff/diagnostic_report.py` | derived-ES review | Mid | Low | @@ -74,15 +74,12 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m | Reuse the demeaner's factorized codes in `absorbed_fe_rank`/`absorbed_fe_cr1_k_increment` instead of re-factorizing: at 186k rows the rank helper adds ~1.9 ms per absorbed fit (7.7% of the fastest Rust-served TWFE fit) and the K_reference increment ~3.2 ms per clustered-hc1 absorbed fit (~13%; see `docs/performance-plan.md` "Component-aware absorbed-FE rank"), and the helpers and `demean_by_groups` factorize the same group columns. Threading the codes through the call sites halves the factorize work; the `connected_components` call itself is ~1.1 ms. Deliberately not done in the correctness PRs. | `diff_diff/utils.py` | #variance-inventory | Quick | Low | | `EfficientDiD` conditional path: the largest remaining O(n) stage is the sieve/nuisance construction outside the tiled pass (~9s at 10k). (The `_ridge_solve_weights` Python-prep shave landed 2026-07-07 — the `omega_stack[rest]` fancy-index copy and tail scatter are skipped when no row is zero-masked, byte-identical outputs; the `zero_mask` abs scan itself remains, needed for correctness.) | `efficient_did_covariates.py` | CS-scaling | Mid | Low | | `_rq_fit` LP assembly is dense (`A_eq = [X, I, -I]` with dense identity blocks, rebuilt per cell fit): a `scipy.sparse` construction would cut memory and likely HiGHS time for large cells / bootstrap-heavy covariate CiC/QDiD fits. CAVEAT before doing it: a different matrix representation can change HiGHS's vertex selection at degenerate/tied QR optima - end-to-end covariate goldens are tie-selection-gated (fine), but the `qr_cases` tight coefficient matches may shift to the equal-loss branch; re-run the parity suite and re-calibrate if needed. | `diff_diff/changes_in_changes.py::_rq_fit` | covariates PR | Quick | Low | -| `_compute_dml_rcs_gt` computes the Case 2 λ-slope `Ĝ₂λ` twice per successful cell — once inside `chang_rcs_score_augmented()` and again for the `g2_lambda` diagnostic — duplicating the input validation and an O(n_cell) pass (nuisance fitting still dominates). Add a private augmented-score helper that accepts a precomputed slope, or return `(psi_bar, g2_lambda)` from an internal variant, keeping the public API unchanged | `diff_diff/_dr_scores.py`, `diff_diff/dml_did.py` | #794 | Quick | Low | | Evaluate flipping `DIFF_DIFF_SOLVE_OLS_FASTPATH` default-ON after an opt-in soak (the 2026-07 certified normal-equations Cholesky fast path, both backends). A flip needs: golden/parity-suite recapture at the tol-bounded posture (fitted ~1e-8 abs / SE ~1e-6 rel — the default today is byte-pinned in several benchmark conventions), certification-rate telemetry across real workloads (any decline is silent-correct but forfeits the speedup), and the staged default-flip protocol used for `df_convention` (v4-class change). Lifecycle tracked in docs/v4-deprecations.yaml (M-008). | `diff_diff/linalg.py::_resolve_solve_ols_fastpath`, `rust/src/linalg.rs::solve_ols_chol` | CS-scaling | Mid | Low | ### Testing / docs | Issue | Location | Origin | Effort | Priority | |-------|----------|--------|--------|----------| -| `CallawaySantAnnaResults.summary(alpha=...)` (and sibling staggered summaries relaying the parent renderer) relabels the confidence-interval header at the requested alpha while always printing the FIT-TIME `overall_conf_int` — silent mislabeling on any non-fit alpha (bootstrap percentile intervals cannot be reconstructed from the SE at all). DMLDiDResults now rejects non-fit alpha (the EventStudyResults.summary precedent, results_base.py:643-660); apply the same guard (or a real recomputation on analytical fits) to the CS-family summaries | `diff_diff/staggered_results.py` | DML PR-B1 review | Quick | Medium | -| `plot_event_study`'s pointwise CI reconstruction (`_event_study.py:302-304`) gates on `np.isfinite(std_err)` but not `> 0` — a zero-SE period draws a finite zero-width interval while its stored inference is all-NaN (the CS-surface twin of the `plot_group_effects` alternate-alpha gate fixed in DML PR-B1; pre-existing main behavior, cross-surface-twins audit) — apply the same `se > 0` NaN-gate | `diff_diff/visualization/_event_study.py` | DML PR-B1 review | Quick | Medium | | DMLDiD tutorial notebook (CONTRIBUTING's new-estimator checklist requires a tutorial; deferred to its own PR per the numbers-locked notebook protocol — prototype in scripts, lock numbers, assemble + execute once, register in `docs/tutorials/index.rst` with a toctree short label + group card): staggered DGP with nonlinear covariate confounding, learner comparison (linear/ridge/sieve/sklearn object), post-fit aggregation + HonestDiD via the event-study container, seed/reproducibility note, and a `panel=False` repeated-cross-section example | `docs/tutorials/`, `docs/tutorials/index.rst` | DML PR-B1 | Mid | Medium | | Replicate Chang (2020) §4's own RCS simulation DGPs (pp. 17-21, "fully specified" per the paper review) as recovery/coverage fixtures for the `DMLDiD(panel=False)` lane — the shipped tests use a library-authored RCS design (documented in the REGISTRY checklist caveat); needs the paper PDF to extract the parameterization | `tests/test_methodology_dml_did.py`, `docs/methodology/papers/chang-2020-review.md` | DML PR-B2 | Mid | Low | | Optional scheduled end-to-end execution gate for the MMM tutorials (29/30): a cron-only workflow (or extension of `mmm-interop.yml`) that executes both notebooks in isolated exact-pin environments, so a stale/invalid committed posterior cannot stay green indefinitely - today the hybrid posture (deliberate: notebooks execute locally with committed outputs; CI smoke-tests the exporters without sampling; drift tests pin source + committed-output needles) leaves the MCMC claims un-re-executed in CI | `.github/workflows/mmm-interop.yml`, `docs/tutorials/29_mmm_calibration_pymc.ipynb`, `docs/tutorials/30_mmm_calibration_meridian.ipynb` | mmm-interop | Mid | Low | diff --git a/diff_diff/_dr_scores.py b/diff_diff/_dr_scores.py index 043ce1320..806e9a226 100644 --- a/diff_diff/_dr_scores.py +++ b/diff_diff/_dr_scores.py @@ -373,6 +373,19 @@ def chang_rcs_lambda_slope( y, D, T, m2_hat, ps = _validate_chang_rcs_inputs( y, D, T, m2_hat, ps, p_hat, lam_hat, "chang_rcs_lambda_slope" ) + return _chang_rcs_lambda_slope_validated(y, D, T, m2_hat, ps, p_hat, lam_hat) + + +def _chang_rcs_lambda_slope_validated( + y: np.ndarray, + D: np.ndarray, + T: np.ndarray, + m2_hat: np.ndarray, + ps: np.ndarray, + p_hat: float, + lam_hat: float, +) -> float: + # Assumes inputs already coerced/validated by _validate_chang_rcs_inputs. odds = (D - ps) / (1.0 - ps) term1 = ( -((1.0 - 2.0 * lam_hat) / (lam_hat**2 * (1.0 - lam_hat) ** 2)) @@ -400,7 +413,9 @@ def chang_rcs_score_augmented( (T_i - lam_hat)`` — Theorem 2's combined score: the treated-share correction ``G_2p = -theta/p_hat`` folds into the score exactly as in Case 1, while the lambda-correction stays an EXPLICIT extra term - (``G_2lambda`` computed internally via :func:`chang_rcs_lambda_slope`). + (``G_2lambda`` computed internally via + :func:`_chang_rcs_lambda_slope_validated`, the shared slope kernel + behind :func:`chang_rcs_lambda_slope`). The variance estimator is ``SE = sqrt(mean(psi_bar**2) / N)``. Per the methodology review: "Omitting the λ-correction term is a @@ -409,6 +424,29 @@ def chang_rcs_score_augmented( parity anchor exists for this object (``DoubleMLDIDCSBinary``'s variance omits the lambda term; see the committed characterization spike). """ + return _chang_rcs_score_augmented_with_slope( + summand, D, T, y, m2_hat, ps, theta, p_hat, lam_hat + )[0] + + +def _chang_rcs_score_augmented_with_slope( + summand: np.ndarray, + D: np.ndarray, + T: np.ndarray, + y: np.ndarray, + m2_hat: np.ndarray, + ps: np.ndarray, + theta: float, + p_hat: float, + lam_hat: float, +) -> Tuple[np.ndarray, float]: + """Internal variant returning ``(psi_bar, g2_lambda)``. + + Validates once and computes the O(n) lambda-slope pass once, so a + caller needing both the augmented score and the ``G_2lambda`` + diagnostic (the DMLDiD RCS cell loop) avoids the duplicate + validation + slope pass of calling the two public functions. + """ context = "chang_rcs_score_augmented" summand = np.asarray(summand, dtype=np.float64) if summand.ndim != 1: @@ -422,5 +460,5 @@ def chang_rcs_score_augmented( y, D, T, m2_hat, ps = _validate_chang_rcs_inputs(y, D, T, m2_hat, ps, p_hat, lam_hat, context) if summand.shape[0] != y.shape[0]: raise ValueError(f"{context}: summand has length {summand.shape[0]}, expected {y.shape[0]}") - g2_lambda = chang_rcs_lambda_slope(y, D, T, m2_hat, ps, p_hat, lam_hat) - return summand - D * theta / p_hat + g2_lambda * (T - lam_hat) + g2_lambda = _chang_rcs_lambda_slope_validated(y, D, T, m2_hat, ps, p_hat, lam_hat) + return summand - D * theta / p_hat + g2_lambda * (T - lam_hat), g2_lambda diff --git a/diff_diff/chaisemartin_dhaultfoeuille_results.py b/diff_diff/chaisemartin_dhaultfoeuille_results.py index ff1712c45..74299e852 100644 --- a/diff_diff/chaisemartin_dhaultfoeuille_results.py +++ b/diff_diff/chaisemartin_dhaultfoeuille_results.py @@ -34,7 +34,7 @@ from diff_diff._deprecation import deprecated_field_property from diff_diff.aggregation import AggregationMixin, AggregationResult from diff_diff.results import _get_significance_stars -from diff_diff.results_base import BaseResults, build_event_study_surface +from diff_diff.results_base import BaseResults, _require_fit_alpha, build_event_study_surface __all__ = [ "ChaisemartinDHaultfoeuilleResults", @@ -920,8 +920,12 @@ def summary(self, alpha: Optional[float] = None) -> str: Parameters ---------- alpha : float, optional - Significance level for the confidence interval header. Defaults - to ``self.alpha``. + Accepted for signature uniformity. The stored intervals were + computed at fit time; a value different from the stored + ``alpha`` raises ValueError rather than silently recomputing + or relabeling (bootstrap percentile intervals cannot be + reconstructed from the reported SE). Re-fit at the desired + alpha instead. Returns ------- @@ -930,7 +934,7 @@ def summary(self, alpha: Optional[float] = None) -> str: joiners-only / leavers-only views, the placebo, the TWFE decomposition diagnostic, and a footer of significance codes. """ - alpha = alpha or self.alpha + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) width = 85 sep = "=" * width diff --git a/diff_diff/continuous_did.py b/diff_diff/continuous_did.py index 1e4b6390a..83ba436cf 100644 --- a/diff_diff/continuous_did.py +++ b/diff_diff/continuous_did.py @@ -46,7 +46,12 @@ build_unit_first_row_index, compute_survey_vcov, ) -from diff_diff.utils import safe_inference, validate_anticipation, validate_n_bootstrap +from diff_diff.utils import ( + safe_inference, + validate_anticipation, + validate_n_bootstrap, + validate_pscore_trim, +) if TYPE_CHECKING: from diff_diff.survey import ResolvedSurveyDesign, SurveyDesign @@ -218,7 +223,7 @@ class ContinuousDiD(_ContinuousDiDAggregationMixin, BaseEstimator): ``overall_att`` / ATT(d) level and in its doubly-robust standard errors. pscore_trim : float, default=0.01 Propensity-score trimming bound for the ``dr`` path (scores clipped to - ``[pscore_trim, 1 - pscore_trim]``). + ``[pscore_trim, 1 - pscore_trim]``). Must be in ``(0, 0.5)``. epv_threshold : float, default=10.0 Events-per-variable threshold for the ``dr`` propensity logit diagnostics. pscore_fallback : str, default="error" @@ -339,10 +344,9 @@ def _validate_constrained_params(self) -> None: f"Invalid pscore_fallback: '{self.pscore_fallback}'. " "Must be 'unconditional' or 'error'." ) - if not (np.isfinite(self.pscore_trim) and 0.0 <= self.pscore_trim < 0.5): - raise ValueError( - f"Invalid pscore_trim: {self.pscore_trim}. " "Must be finite and in [0, 0.5)." - ) + # Shared helper (utils.validate_pscore_trim): rejects 0 (which would + # disable the overlap clip) and non-real-scalar inputs, coerces to float. + self.pscore_trim = validate_pscore_trim(self.pscore_trim) if not (np.isfinite(self.epv_threshold) and self.epv_threshold > 0): raise ValueError( f"Invalid epv_threshold: {self.epv_threshold}. Must be finite and > 0." diff --git a/diff_diff/dml_did.py b/diff_diff/dml_did.py index a0de7a09d..ec9b3c7b9 100644 --- a/diff_diff/dml_did.py +++ b/diff_diff/dml_did.py @@ -39,11 +39,10 @@ from diff_diff._base import BaseEstimator from diff_diff._crossfit import DegenerateFoldError, assign_folds, cross_fit_predict from diff_diff._dr_scores import ( + _chang_rcs_score_augmented_with_slope, chang_panel_score, chang_panel_score_augmented, - chang_rcs_lambda_slope, chang_rcs_score, - chang_rcs_score_augmented, ) from diff_diff._learners import ( _CLASSIFIER_NAMES, @@ -70,6 +69,7 @@ validate_anticipation, validate_covariate_names, validate_n_bootstrap, + validate_pscore_trim, ) __all__ = ["DMLDiD"] @@ -83,18 +83,6 @@ _LABEL_MAGNITUDE_BOUND = 2**62 -def _validate_pscore_trim(value: Any) -> float: - """Type guard BEFORE the range comparison (TripleDifference precedent).""" - if isinstance(value, bool) or not isinstance(value, (int, float, np.integer, np.floating)): - raise ValueError( - f"pscore_trim must be a real number in (0, 0.5), got {value!r} " - f"(type {type(value).__name__})" - ) - if not np.isfinite(value) or not 0 < value < 0.5: - raise ValueError(f"pscore_trim must be in (0, 0.5), got {value}") - return float(value) - - def _validate_n_folds(value: Any) -> int: if isinstance(value, bool) or not isinstance(value, (int, np.integer)): raise ValueError(f"n_folds must be an integer >= 2, got {value!r}") @@ -399,7 +387,7 @@ def _revalidate_config(self) -> None: "would silently enable bands" ) self.cband = bool(self.cband) - self.pscore_trim = _validate_pscore_trim(self.pscore_trim) + self.pscore_trim = validate_pscore_trim(self.pscore_trim) if not isinstance(self.panel, (bool, np.bool_)): raise ValueError( f"panel must be a bool, got {self.panel!r} (type " @@ -1420,12 +1408,9 @@ def _compute_dml_rcs_gt( with np.errstate(over="ignore", invalid="ignore"): summand = chang_rcs_score(y_cell, D_cell, T_cell, m2_hat, ps, p_hat, lam_hat) theta = float(np.mean(summand)) - psi_bar = chang_rcs_score_augmented( + psi_bar, g2_lambda = _chang_rcs_score_augmented_with_slope( summand, D_cell, T_cell, y_cell, m2_hat, ps, theta, p_hat, lam_hat ) - g2_lambda = chang_rcs_lambda_slope( - y_cell, D_cell, T_cell, m2_hat, ps, p_hat, lam_hat - ) se = float(np.sqrt(np.mean(psi_bar**2) / n_cell)) except ValueError as exc: diagnostics["skip_reason"] = "non_finite_score" diff --git a/diff_diff/efficient_did_results.py b/diff_diff/efficient_did_results.py index 36ea67d9e..82df5d08f 100644 --- a/diff_diff/efficient_did_results.py +++ b/diff_diff/efficient_did_results.py @@ -21,7 +21,7 @@ from diff_diff.efficient_did_aggregation import _EfficientAggregationMixin from diff_diff.efficient_did_bootstrap import EfficientDiDBootstrapMixin from diff_diff.results import _format_survey_block, _get_significance_stars -from diff_diff.results_base import BaseResults, build_event_study_surface +from diff_diff.results_base import BaseResults, _require_fit_alpha, build_event_study_surface if TYPE_CHECKING: from diff_diff.efficient_did_bootstrap import EDiDBootstrapResults @@ -622,8 +622,13 @@ def coef_var(self) -> float: return self.overall_se / abs(self.overall_att) def summary(self, alpha: Optional[float] = None) -> str: - """Generate formatted summary of estimation results.""" - alpha = alpha or self.alpha + """Generate formatted summary of estimation results. + + ``alpha`` is accepted for signature uniformity; a value different + from the fit-time ``alpha`` raises ValueError (stored inference is + never recomputed or relabeled - re-fit at the desired alpha). + """ + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) lines = [ diff --git a/diff_diff/imputation_results.py b/diff_diff/imputation_results.py index 3881b8279..7b4f46708 100644 --- a/diff_diff/imputation_results.py +++ b/diff_diff/imputation_results.py @@ -15,7 +15,7 @@ from diff_diff.aggregation import AggregationMixin, AggregationResult, build_total_relay_row from diff_diff.imputation_aggregation import _ImputationAggregationMixin from diff_diff.results import _format_survey_block, _get_significance_stars -from diff_diff.results_base import BaseResults, build_event_study_surface +from diff_diff.results_base import BaseResults, _require_fit_alpha, build_event_study_surface class _ImputationKitAggregator(_ImputationAggregationMixin): @@ -533,14 +533,19 @@ def summary(self, alpha: Optional[float] = None) -> str: Parameters ---------- alpha : float, optional - Significance level. Defaults to alpha used in estimation. + Accepted for signature uniformity. The stored intervals were + computed at fit time; a value different from the stored + ``alpha`` raises ValueError rather than silently recomputing + or relabeling (bootstrap percentile intervals cannot be + reconstructed from the reported SE). Re-fit at the desired + alpha instead. Returns ------- str Formatted summary. """ - alpha = alpha or self.alpha + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) lines = [ diff --git a/diff_diff/lwdid.py b/diff_diff/lwdid.py index 14bc6efdf..f1bbde0a3 100644 --- a/diff_diff/lwdid.py +++ b/diff_diff/lwdid.py @@ -27,7 +27,7 @@ from diff_diff._base import BaseEstimator from diff_diff.linalg import _detect_rank_deficiency, solve_logit, solve_ols from diff_diff.lwdid_results import LWDiDResults -from diff_diff.utils import safe_inference, validate_binary +from diff_diff.utils import safe_inference, validate_binary, validate_pscore_trim _VALID_ROLLING = ("demean", "detrend", "demeanq", "detrendq") _VALID_ESTIMATION_METHODS = ("reg", "ipw", "dr", "psm") @@ -626,13 +626,7 @@ def __init__( # Engineering parameters (validated, never silently coerced - # review finding: fractional n_neighbors truncated, strings became # with_replacement=True, negative calipers matched nothing) - if not isinstance(pscore_trim, (int, float, np.integer, np.floating)) or isinstance( - pscore_trim, bool - ): - raise ValueError(f"pscore_trim must be a number, got {pscore_trim!r}") - self.pscore_trim = float(pscore_trim) - if not np.isfinite(self.pscore_trim) or not (0.0 < self.pscore_trim < 0.5): - raise ValueError("pscore_trim must be between 0 and 0.5") + self.pscore_trim = validate_pscore_trim(pscore_trim) if not isinstance(n_neighbors, (int, np.integer)) or isinstance(n_neighbors, bool): raise ValueError(f"n_neighbors must be an integer, got {n_neighbors!r}") self.n_neighbors = int(n_neighbors) diff --git a/diff_diff/results_base.py b/diff_diff/results_base.py index acbc7248e..0b0962ef0 100644 --- a/diff_diff/results_base.py +++ b/diff_diff/results_base.py @@ -92,6 +92,26 @@ class BaseResults: __slots__ = () +def _require_fit_alpha(alpha: Optional[float], fit_alpha: float) -> float: + """Reject a non-fit ``alpha``; summaries never recompute stored inference. + + Shared by the staggered-family ``summary()`` methods (CallawaySantAnna + and siblings): stored intervals were computed at fit time, and bootstrap + percentile intervals cannot be reconstructed from the reported SE, so a + requested alpha other than the fit alpha raises instead of silently + relabeling the confidence-interval header. + """ + if alpha is not None and alpha != fit_alpha: + raise ValueError( + f"This result stores intervals computed at alpha={fit_alpha}; " + f"summary() never recomputes or relabels stored inference " + f"(requested alpha={alpha}). Re-fit with the desired alpha " + "(bootstrap percentile intervals cannot be reconstructed from " + "the reported SE)." + ) + return fit_alpha + + def _json_safe_label(value: Any) -> Any: """Convert an event-time label to a JSON-serializable form. diff --git a/diff_diff/stacked_did_results.py b/diff_diff/stacked_did_results.py index 89c810063..30073c755 100644 --- a/diff_diff/stacked_did_results.py +++ b/diff_diff/stacked_did_results.py @@ -14,7 +14,7 @@ from diff_diff._deprecation import deprecated_field_property from diff_diff.aggregation import AggregationMixin, AggregationResult from diff_diff.results import _format_survey_block, _get_significance_stars -from diff_diff.results_base import BaseResults, build_event_study_surface +from diff_diff.results_base import BaseResults, _require_fit_alpha, build_event_study_surface __all__ = [ "StackedDiDResults", @@ -314,14 +314,19 @@ def summary(self, alpha: Optional[float] = None) -> str: Parameters ---------- alpha : float, optional - Significance level. Defaults to alpha used in estimation. + Accepted for signature uniformity. The stored intervals were + computed at fit time; a value different from the stored + ``alpha`` raises ValueError rather than silently recomputing + or relabeling (bootstrap percentile intervals cannot be + reconstructed from the reported SE). Re-fit at the desired + alpha instead. Returns ------- str Formatted summary. """ - alpha = alpha or self.alpha + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) lines = [ diff --git a/diff_diff/staggered.py b/diff_diff/staggered.py index 2dfac0cb2..a859c1b2f 100644 --- a/diff_diff/staggered.py +++ b/diff_diff/staggered.py @@ -49,6 +49,7 @@ safe_inference_batch, validate_anticipation, validate_n_bootstrap, + validate_pscore_trim, ) if TYPE_CHECKING: @@ -612,8 +613,7 @@ def __init__( raise ValueError( f"estimation_method must be 'dr', 'ipw', or 'reg', " f"got '{estimation_method}'" ) - if not (0 < pscore_trim < 0.5): - raise ValueError(f"pscore_trim must be in (0, 0.5), got {pscore_trim}") + pscore_trim = validate_pscore_trim(pscore_trim) if epv_threshold <= 0: raise ValueError(f"epv_threshold must be > 0, got {epv_threshold}") if pscore_fallback not in ["error", "unconditional"]: @@ -1968,9 +1968,9 @@ def fit( if isinstance(balance_e, _DeprecatedFitArg): balance_e = None - # Validate pscore_trim (may have been changed via set_params) - if not (0 < self.pscore_trim < 0.5): - raise ValueError(f"pscore_trim must be in (0, 0.5), got {self.pscore_trim}") + # Validate pscore_trim (may have been changed by direct mutation). + # Return discarded: fit() must not mutate constructor config. + validate_pscore_trim(self.pscore_trim) # NB: the event-study VCV and its df provenance used to be reset here, # because ``_aggregate_event_study`` stashed them on ``self`` and a diff --git a/diff_diff/staggered_results.py b/diff_diff/staggered_results.py index 4363cce1f..3e8fa7103 100644 --- a/diff_diff/staggered_results.py +++ b/diff_diff/staggered_results.py @@ -23,7 +23,7 @@ apply_bootstrap_group_overrides, ) from diff_diff.results import _format_survey_block, _get_significance_stars -from diff_diff.results_base import BaseResults, build_event_study_surface +from diff_diff.results_base import BaseResults, _require_fit_alpha, build_event_study_surface from diff_diff.staggered_aggregation import ( CallawaySantAnnaAggregationMixin, fixed_cohort_agg_weights, @@ -703,14 +703,19 @@ def summary(self, alpha: Optional[float] = None) -> str: Parameters ---------- alpha : float, optional - Significance level. Defaults to alpha used in estimation. + Accepted for signature uniformity. The stored intervals were + computed at fit time; a value different from the stored + ``alpha`` raises ValueError rather than silently recomputing + or relabeling (bootstrap percentile intervals cannot be + reconstructed from the reported SE). Re-fit at the desired + alpha instead. Returns ------- str Formatted summary. """ - alpha = alpha or self.alpha + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) lines = [ diff --git a/diff_diff/staggered_triple_diff_results.py b/diff_diff/staggered_triple_diff_results.py index f2942cbd0..22c3e6be4 100644 --- a/diff_diff/staggered_triple_diff_results.py +++ b/diff_diff/staggered_triple_diff_results.py @@ -12,7 +12,7 @@ import pandas as pd from diff_diff.results import _format_survey_block, _get_significance_stars -from diff_diff.results_base import BaseResults +from diff_diff.results_base import BaseResults, _require_fit_alpha if TYPE_CHECKING: from diff_diff.staggered_bootstrap import CSBootstrapResults @@ -162,14 +162,19 @@ def summary(self, alpha: Optional[float] = None) -> str: Parameters ---------- alpha : float, optional - Significance level. Defaults to alpha used in estimation. + Accepted for signature uniformity. The stored intervals were + computed at fit time; a value different from the stored + ``alpha`` raises ValueError rather than silently recomputing + or relabeling (bootstrap percentile intervals cannot be + reconstructed from the reported SE). Re-fit at the desired + alpha instead. Returns ------- str Formatted summary. """ - alpha = alpha or self.alpha + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) lines = [ diff --git a/diff_diff/sun_abraham.py b/diff_diff/sun_abraham.py index 7e7ff5b92..37409a9ba 100644 --- a/diff_diff/sun_abraham.py +++ b/diff_diff/sun_abraham.py @@ -34,7 +34,7 @@ from diff_diff.survey import ResolvedSurveyDesign, SurveyDesign from diff_diff.linalg import LinearRegression from diff_diff.results import _format_survey_block, _get_significance_stars -from diff_diff.results_base import BaseResults +from diff_diff.results_base import BaseResults, _require_fit_alpha from diff_diff.utils import ( absorbed_fe_cr1_k_increment, absorbed_fe_rank, @@ -263,14 +263,19 @@ def summary(self, alpha: Optional[float] = None) -> str: Parameters ---------- alpha : float, optional - Significance level. Defaults to alpha used in estimation. + Accepted for signature uniformity. The stored intervals were + computed at fit time; a value different from the stored + ``alpha`` raises ValueError rather than silently recomputing + or relabeling (bootstrap percentile intervals cannot be + reconstructed from the reported SE). Re-fit at the desired + alpha instead. Returns ------- str Formatted summary. """ - alpha = alpha or self.alpha + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) lines = [ diff --git a/diff_diff/triple_diff.py b/diff_diff/triple_diff.py index 67e48f2b9..a43c6c962 100644 --- a/diff_diff/triple_diff.py +++ b/diff_diff/triple_diff.py @@ -53,6 +53,7 @@ staggered_ddd_ctor_offenders, validate_anticipation, validate_n_bootstrap, + validate_pscore_trim, ) if TYPE_CHECKING: @@ -649,27 +650,10 @@ def __init__( f"got '{bootstrap_weights}'" ) validate_n_bootstrap(n_bootstrap) - # pscore_trim gains the staggered engine's range check (row M-142). It - # was previously unvalidated here, but the value feeds - # np.clip(pscore, trim, 1 - trim) in both engines: trim=0 disables the - # overlap guard that keeps the 1/(1-p) IPW/DR weights finite, and - # trim >= 0.5 inverts the clip bounds. - # The TYPE guard precedes the range check so the documented ValueError is - # what users actually see: a bare `0 < x < 0.5` raises an incidental - # TypeError on None/str/complex/list, an ambiguous-truth ValueError on a - # multi-element array, and - worst - ACCEPTS a 1-element array, storing an - # ndarray as the parameter. Same shape as validate_n_bootstrap: reject - # bool (True would read as a 1.0 trim), then non-real-scalar, then - # non-finite, then the range. - if isinstance(pscore_trim, bool) or not isinstance( - pscore_trim, (int, float, np.integer, np.floating) - ): - raise ValueError( - f"pscore_trim must be a real number in (0, 0.5), got {pscore_trim!r} " - f"(type {type(pscore_trim).__name__})" - ) - if not np.isfinite(pscore_trim) or not 0 < pscore_trim < 0.5: - raise ValueError(f"pscore_trim must be in (0, 0.5), got {pscore_trim}") + # pscore_trim range check per row M-142 (trim=0 disables the overlap + # guard, trim >= 0.5 inverts the clip bounds); type guard + coercion + # via the shared utils.validate_pscore_trim helper. + pscore_trim = validate_pscore_trim(pscore_trim) if rank_deficient_action not in ["warn", "error", "silent"]: raise ValueError( f"rank_deficient_action must be 'warn', 'error', or 'silent', " diff --git a/diff_diff/two_stage_results.py b/diff_diff/two_stage_results.py index 39c24d7f2..dd4554173 100644 --- a/diff_diff/two_stage_results.py +++ b/diff_diff/two_stage_results.py @@ -14,7 +14,7 @@ from diff_diff.aggregation import AggregationMixin, AggregationResult, build_total_relay_row from diff_diff.results import _format_survey_block, _get_significance_stars -from diff_diff.results_base import BaseResults, build_event_study_surface +from diff_diff.results_base import BaseResults, _require_fit_alpha, build_event_study_surface from diff_diff.two_stage_aggregation import _TwoStageAggregationMixin @@ -532,14 +532,19 @@ def summary(self, alpha: Optional[float] = None) -> str: Parameters ---------- alpha : float, optional - Significance level. Defaults to alpha used in estimation. + Accepted for signature uniformity. The stored intervals were + computed at fit time; a value different from the stored + ``alpha`` raises ValueError rather than silently recomputing + or relabeling (bootstrap percentile intervals cannot be + reconstructed from the reported SE). Re-fit at the desired + alpha instead. Returns ------- str Formatted summary. """ - alpha = alpha or self.alpha + alpha = _require_fit_alpha(alpha, self.alpha) conf_level = int((1 - alpha) * 100) lines = [ diff --git a/diff_diff/utils.py b/diff_diff/utils.py index 42c53f0c3..d93df96f4 100644 --- a/diff_diff/utils.py +++ b/diff_diff/utils.py @@ -503,6 +503,31 @@ def validate_n_bootstrap(n_bootstrap: Any) -> None: raise ValueError(f"n_bootstrap must be a non-negative integer, got '{n_bootstrap}'") +def validate_pscore_trim(value: Any) -> float: + """Validate ``pscore_trim`` and return it coerced to ``float``. + + Shared by every estimator whose ``pscore_trim`` feeds + ``np.clip(pscore, trim, 1 - trim)`` (TripleDifference, DMLDiD, + ContinuousDiD, LWDiD, CallawaySantAnna): ``trim=0`` disables the + overlap guard that keeps the ``1/(1-p)`` IPW/DR weights finite, and + ``trim >= 0.5`` inverts the clip bounds. The TYPE guard precedes the + range check (validate_n_bootstrap shape): a bare ``0 < x < 0.5`` + raises an incidental TypeError on None/str/complex, an + ambiguous-truth error on a multi-element array, and ACCEPTS a + 1-element array. StaggeredTripleDifference deliberately keeps its + bare range check (construction-permissive dying class, ledger + M-013/M-144). + """ + if isinstance(value, bool) or not isinstance(value, (int, float, np.integer, np.floating)): + raise ValueError( + f"pscore_trim must be a real number in (0, 0.5), got {value!r} " + f"(type {type(value).__name__})" + ) + if not np.isfinite(value) or not 0 < value < 0.5: + raise ValueError(f"pscore_trim must be in (0, 0.5), got {value}") + return float(value) + + # The staggered-only TripleDifference constructor params that genuinely select # staggered behavior (row M-013). Lives here because TWO independent consumers # need the same boundary and must not drift apart: TripleDifference.fit(), which diff --git a/diff_diff/visualization/_event_study.py b/diff_diff/visualization/_event_study.py index a59da41bc..d3ef0ad96 100644 --- a/diff_diff/visualization/_event_study.py +++ b/diff_diff/visualization/_event_study.py @@ -299,7 +299,18 @@ def plot_event_study( ci_lower = ci_lower_override[period] assert ci_upper_override is not None ci_upper = ci_upper_override[period] - elif np.isfinite(std_err): + elif np.isfinite(std_err) and ( + std_err > 0 or (period == reference_period and effect == 0.0 and std_err == 0.0) + ): + # A zero/negative-SE row has all-NaN stored inference + # (safe_inference), and effect +/- z*0 would draw a finite + # zero-width interval for it - the prohibited partial-NaN + # pattern (plot_group_effects twin). The one retention: an + # auto-inferred reference row (effect exactly 0, se 0) keeps + # its degenerate constraint bar per the REGISTRY Event Study + # Plotting contract; the effect == 0.0 conjunct stops the + # unconditional -1 reference fallback from retaining a + # genuinely estimated zero-SE row. ci_lower = effect - critical_value * std_err ci_upper = effect + critical_value * std_err else: @@ -1061,6 +1072,46 @@ def _extract_plot_data( ) +def _honest_raw_route_periods( + periods: Optional[List[Any]], + effects_dict: Dict[Any, float], + se_dict: Dict[Any, float], + reference_period: Optional[Any], +) -> List[Any]: + """Period roster for the raw (non-container) honest-plot routes. + + Mirrors the container route's ``_retained`` semantics: rows with + undefined inference (non-finite or zero SE) carry no honest interval + and would otherwise be painted with a zero-width original CI plus the + aggregate honest interval (or KeyError on per-period bounds), so they + are excluded up front; the reference row is kept as a + normalization-only anchor. + """ + + def _defined(p: Any) -> bool: + s = se_dict.get(p, float("nan")) + return bool(np.isfinite(s) and float(s) > 0) + + if periods is None: + retained = [p for p in sorted(effects_dict.keys()) if _defined(p) or p == reference_period] + else: + _bad = [p for p in periods if p in se_dict and not _defined(p) and p != reference_period] + if _bad: + raise ValueError( + f"Requested periods {_bad} have undefined inference " + "(non-finite or zero SE) on this event-study surface: " + "HonestDiD excludes such rows from the sensitivity " + "analysis and they carry no honest interval." + ) + retained = list(periods) + if not retained: + raise ValueError( + "No valid data to plot: every period on this event-study " + "surface has undefined inference (non-finite or zero SE)." + ) + return retained + + def plot_honest_event_study( honest_results: "HonestDiDResults", *, @@ -1090,9 +1141,14 @@ def plot_honest_event_study( honest_results : HonestDiDResults Results from HonestDiD.fit() that include event_study_bounds. periods : list, optional - Periods to plot. If None, uses all available periods. + Periods to plot. If None, uses all periods with defined inference + (non-container routes exclude zero/non-finite-SE rows, which carry + no honest interval; explicitly requesting one raises ValueError). reference_period : any, optional - Reference period to show as hollow marker. + Reference period to show as hollow marker. If None, inferred from + the results object where possible (a ``reference_period`` + attribute, the container's reference marks, or a + normalization-constraint row on fit-time dict surfaces). figsize : tuple, default=(10, 6) Figure size. title : str @@ -1188,16 +1244,32 @@ def plot_honest_event_study( # MultiPeriodDiDResults effects_dict = {p: pe.effect for p, pe in original_results.period_effects.items()} se_dict = {p: pe.se for p, pe in original_results.period_effects.items()} - if periods is None: - periods = list(original_results.period_effects.keys()) + if reference_period is None: + reference_period = getattr(original_results, "reference_period", None) + periods = _honest_raw_route_periods(periods, effects_dict, se_dict, reference_period) elif hasattr(original_results, "event_study_effects"): - # CallawaySantAnnaResults + # CallawaySantAnnaResults (fit-time dict surface) effects_dict = { t: data["effect"] for t, data in original_results.event_study_effects.items() } se_dict = {t: data["se"] for t, data in original_results.event_study_effects.items()} - if periods is None: - periods = sorted(original_results.event_study_effects.keys()) + if reference_period is None: + reference_period = getattr(original_results, "reference_period", None) + if reference_period is None: + # HonestDiD's own reference signature (honest_did.py): a + # normalization-constraint row, NOT any row whose inference + # happens to be undefined. No signature match (e.g. a + # marker-less varying-base surface) means no inferred + # reference - deliberately no -1 fallback here. + for t, data in original_results.event_study_effects.items(): + if ( + data.get("n_groups", data.get("n_obs", 1)) == 0 + and data["effect"] == 0.0 + and not np.isfinite(data["se"]) + ): + reference_period = t + break + periods = _honest_raw_route_periods(periods, effects_dict, se_dict, reference_period) else: raise TypeError("Cannot extract event study data from original_results") diff --git a/docs/methodology/REGISTRY.md b/docs/methodology/REGISTRY.md index bb9483fca..42bf8c2bb 100644 --- a/docs/methodology/REGISTRY.md +++ b/docs/methodology/REGISTRY.md @@ -1203,6 +1203,7 @@ The multiplier bootstrap uses random weights w_i with E[w]=0 and Var(w)=1: - [x] Repeated cross-sections (`panel=False`) for non-panel surveys (Phase 7b) - **Note:** `anticipation` is validated at construction (non-negative integer; `bool` rejected) via the shared `utils.validate_anticipation`, and re-checked on the fit path (direct-mutation defense) — see the family-wide adoption note (ledger row [M-144]) in the TripleDifference staggered-mode section. +- **Note (`summary(alpha=)` never recomputes stored inference, ledger row M-146):** `CallawaySantAnnaResults.summary`/`print_summary` — and the sibling staggered-family results classes (StaggeredTripleDiffResults, ChaisemartinDHaultfoeuilleResults, ImputationDiDResults, EfficientDiDResults, TwoStageDiDResults, StackedDiDResults, SunAbrahamResults) — reject an `alpha` different from the fit-time value via the shared `results_base._require_fit_alpha` guard (the DMLDiDResults/EventStudyResults precedent). Previously the requested alpha relabeled the confidence-interval header while the FIT-TIME stored intervals were printed — silent coverage mislabeling (bootstrap percentile intervals cannot be reconstructed from the SE at all); `alpha=0.0` was additionally swallowed by a falsy-`or` default and now raises. Re-fit at the desired alpha instead. The non-staggered summaries with the same idiom are tracked as a TODO.md audit row. --- ## ChaisemartinDHaultfoeuille @@ -3626,10 +3627,12 @@ shared verbatim. `np.clip(pscore, trim, 1 - trim)` in both engines, so `trim=0` disabled the overlap guard that keeps the `1/(1-p)` IPW/DR weights finite and `trim >= 0.5` inverted the clip bounds. `TripleDifference(pscore_trim=0)` therefore changes - from accepted to a loud `ValueError`. **Sibling divergence, recorded rather than - silently tolerated:** `ContinuousDiD` still validates `0.0 <= pscore_trim < 0.5` - and so still admits `0`; aligning it is out of scope for a DDD merge and is - tracked as a `TODO.md` row. + from accepted to a loud `ValueError`. The sibling divergence this Note + originally recorded (`ContinuousDiD` still validated `0.0 <= pscore_trim < 0.5`, + admitting `0`) was closed by ledger row M-145: every consumer now validates via + the shared `utils.validate_pscore_trim` (`0 < x < 0.5`, type guard, `float` + coercion), with the deprecated `StaggeredTripleDifference` deliberately keeping + its permissive construction shape (M-013/M-144 posture). - **Note (the `triple_difference()` wrapper stays 2x2x2-only):** the deprecated functional wrapper is deliberately NOT extended to staggered mode. It reaches only the 2x2x2 design and forwards its own `time=` as `post=`. Three reasons, @@ -5886,6 +5889,28 @@ should be a deliberate user choice. normalization is accepted only when the chosen period is one of the marked rows (a no-op shift); any other period raises `ValueError`, because each anchor is a constraint under its own cohort base and no single shift represents them faithfully +- **Note (zero-SE pointwise gate, `plot_group_effects` twin):** the pointwise + `effect +/- z*SE` reconstruction NaN-gates zero/negative-SE rows on every path that + reaches it - a zero-SE row has all-NaN stored inference (`safe_inference`), and a finite + zero-width interval would present defined inference for it (the prohibited partial-NaN + pattern). This includes the `EventStudyResults` container route after an explicit + `reference_period=` (which discards the stored-interval overrides): a zero-SE + non-reference container row previously drew a spurious `(0, 0)` bar there and now draws + none. The one retention: on the raw `event_study_effects` dict route, an auto-inferred + reference row (`effect` exactly 0, `se` 0) keeps its degenerate constraint bar per the + "retained for auto-inferred" rule above - the `effect == 0` conjunct stops the `-1` + reference fallback from retaining a genuinely estimated zero-SE row (a degenerate + estimated row with effect exactly 0 and se 0 at the fallback position retains today's + bar - today's behavior preserved, not a new defect). The dCDH route's synthesized + reference carries `se = NaN` and keeps its hollow-marker-no-bar rendering, unchanged. + Plotly caveat: the plotly renderer filters NaN-CI rows out of the CI band and + interpolates the polygon across the gap (pre-existing NaN-SE convention); the matplotlib + backend is where the gated interval visibly disappears. `plot_honest_event_study`'s raw + (non-container) routes mirror the container's retained-row semantics: zero/non-finite-SE + rows are excluded up front (explicitly requesting one raises), the reference row - now + auto-inferred on raw routes from a `reference_period` attribute or HonestDiD's own + constraint signature, never a bare `-1` fallback - is kept as a normalization anchor, + and an all-undefined surface raises instead of rendering a blank figure. **Reference implementation(s):** - R: `fixest::coefplot()` with reference category shown at 0 with no CI diff --git a/docs/migration-4.0.md b/docs/migration-4.0.md index e7c1e5148..54dcda9f3 100644 --- a/docs/migration-4.0.md +++ b/docs/migration-4.0.md @@ -223,8 +223,8 @@ each one with its target. Smaller items that do not fit the families above — two inert `SyntheticDiD` constructor parameters, the `covariates=` constructor-to-`fit()` move, a retired transition warning, the Bacon roster re-homing, and the family-wide `anticipation` validation below. The appendix lists -the ledger-derived removals/flips; [M-144] is a behavior tightening with no removal/deprecation -fields and appears here only. +the ledger-derived removals/flips; [M-144], [M-145], and [M-146] are behavior tightenings with +no removal/deprecation fields and appear here only. - `anticipation` is validated across the family ([M-144], landing at 4.0): whole-valued floats that previously fit identically to their integer now raise — pass the `int`; bool and @@ -234,6 +234,21 @@ fields and appears here only. wording, and its constructor now reports a bad `bootstrap_weights`/`vcov_type`/`df_convention` before a bad `anticipation` (the ordering flipped). +- `pscore_trim` is validated via a shared helper ([M-145], landing at 4.0): `ContinuousDiD` + tightens from `[0, 0.5)` to `(0, 0.5)` — `pscore_trim=0` (which disabled the overlap clip + keeping IPW/DR weights finite) now raises, its message wording changed, and non-real-scalar + inputs (`None`, strings, `Decimal`/`Fraction`, 1-element arrays) raise `ValueError` instead of + `TypeError` (or being silently accepted, for the 1-element array). `CallawaySantAnna` gains the + same type guard; `TripleDifference`, `CallawaySantAnna`, and `ContinuousDiD` now store the value + coerced to built-in `float`; `LWDiD`'s message wording changed. The deprecated + `StaggeredTripleDifference` keeps its permissive construction shape. + +- `summary(alpha=...)` / `print_summary(alpha=...)` on the staggered-family results classes + ([M-146], landing at 4.0): a value different from the fit-time `alpha` now raises `ValueError` + instead of silently relabeling the confidence-interval header over fit-time stored intervals + (bootstrap percentile intervals cannot be reconstructed from the SE); `alpha=0.0`, previously + swallowed by a falsy-`or` default, raises too. Re-fit at the desired alpha instead. + One pending decision: the `DIFF_DIFF_SOLVE_OLS_FASTPATH` environment default has a go/no-go due at 4.0 that has not been made. If it lands on, it is a numerics change and will be documented then; "evaluated, kept off" is an equally valid outcome, so it carries no appendix row today. diff --git a/docs/v4-deprecations.yaml b/docs/v4-deprecations.yaml index 355fd7942..768d3c4a3 100644 --- a/docs/v4-deprecations.yaml +++ b/docs/v4-deprecations.yaml @@ -1718,7 +1718,7 @@ rows: phase: 3 test_ref: tests/test_v4_merge_ddd.py code_refs: [diff_diff/triple_diff.py] - notes: "Input-validation tightening shipped with the M-013 merge: TripleDifference's pscore_trim was previously UNVALIDATED, and the merged constructor adopts the staggered engine's 0 < x < 0.5 rule. Not cosmetic - the value feeds np.clip(pscore, trim, 1 - trim) in both engines, so trim=0 disables the overlap guard that keeps the 1/(1-p) IPW/DR weights finite and trim >= 0.5 inverts the clip bounds. TripleDifference(pscore_trim=0) therefore changes from accepted to a loud ValueError; no in-repo caller passed it. Same shape as [M-096] (a 3.9 validation tightening on a previously-unvalidated/silently-degrading param, rowed in the PR that shipped it), and status 'done' is terminal so the row is exempt from both phase-table directions. SIBLING DIVERGENCE, deliberate and recorded in REGISTRY: ContinuousDiD still validates 0.0 <= pscore_trim < 0.5, i.e. it admits 0 - aligning it is out of scope for a DDD merge and carries a TODO.md row instead of silent drift." + notes: "Input-validation tightening shipped with the M-013 merge: TripleDifference's pscore_trim was previously UNVALIDATED, and the merged constructor adopts the staggered engine's 0 < x < 0.5 rule. Not cosmetic - the value feeds np.clip(pscore, trim, 1 - trim) in both engines, so trim=0 disables the overlap guard that keeps the 1/(1-p) IPW/DR weights finite and trim >= 0.5 inverts the clip bounds. TripleDifference(pscore_trim=0) therefore changes from accepted to a loud ValueError; no in-repo caller passed it. Same shape as [M-096] (a 3.9 validation tightening on a previously-unvalidated/silently-degrading param, rowed in the PR that shipped it), and status 'done' is terminal so the row is exempt from both phase-table directions. SIBLING DIVERGENCE (historical): at the time of this row ContinuousDiD still validated 0.0 <= pscore_trim < 0.5 (admitting 0) with the alignment tracked as a TODO.md row; [M-145] closed that divergence by promoting the shared utils.validate_pscore_trim and aligning ContinuousDiD to 0 < x < 0.5." - id: M-143 kind: field group: merge-qdid @@ -1746,3 +1746,29 @@ rows: test_ref: tests/test_anticipation_policy.py code_refs: [diff_diff/utils.py, diff_diff/staggered.py, diff_diff/sun_abraham.py, diff_diff/imputation.py, diff_diff/two_stage.py, diff_diff/stacked_did.py, diff_diff/continuous_did.py, diff_diff/efficient_did.py, diff_diff/spillover.py, diff_diff/wooldridge.py, diff_diff/triple_diff.py, diff_diff/staggered_triple_diff.py, diff_diff/_staggered_triple_diff_engine.py, diff_diff/guides/llms-full.txt, diff_diff/guides/llms-practitioner.txt, docs/methodology/REGISTRY.md] notes: "Family-wide anticipation domain validation: nine estimators (CS, SA, ImputationDiD, TwoStageDiD, StackedDiD, ContinuousDiD, EfficientDiD, WooldridgeDiD, SpilloverDiD) adopt the shared utils.validate_anticipation at __init__ plus a uniform fit-path mutation re-check in the assignment form (the validator now RETURNS the normalized Python int, adopted via assignment at every call site - constructor AND fit path - so numpy scalars, np.uint64 included, are normalized before any g-1-anticipation arithmetic can overflow). Prior state: seven constructors unvalidated; spillover validated only at the fit path with a bool hole; wooldridge validated >= 0 only, with bool + raw-TypeError-on-None/str holes. Not cosmetic - the value feeds the base-period rule and the NYT threshold: CS anticipation=-1 moved overall_att 2.18 -> 0.34 on the measurement fixture and flipped its sign under control_group='not_yet_treated'; True fit bit-identically to 1 (a silent one-period window); SunAbraham(anticipation=1.5) returned att=NaN without raising. Behavior delta: accepted -> loud ValueError; whole-valued floats that previously fit identically to their int now raise on CS/SA/Imputation/TwoStage/EfficientDiD/Wooldridge; numpy-integer inputs previously survived into the public attribute and get_params() as numpy scalars and are now retyped to built-in int; Wooldridge's message text changed and its constructor error ORDERING moved (the anticipation raise now fires at the assignment, after the bootstrap_weights/vcov_type/df_convention checks); Spillover's raise moved to construction with the fit-path re-check retained. No in-repo caller passed out-of-domain values. introduced_in 4.0: shipped post-3.9.0-cut, so the locked ladder's next release (4.0, tests/test_naming_guard.py _NEXT_RELEASE) is the first release carrying it; rowed per the M-096/M-142 shape precedent; status done is terminal so the row gates nothing further. Deprecated StaggeredTripleDifference stays construction-permissive by design (frozen 3.x shape; the shared engine validates at fit)." + - id: M-145 + kind: behavior + group: policy-pscore-trim + old: "diff_diff:ContinuousDiD[pscore_trim]" + new: null + introduced_in: "4.0" + deprecated_in: null + removed_in: null + status: done + phase: 5 + test_ref: tests/test_continuous_did.py + code_refs: [diff_diff/utils.py, diff_diff/continuous_did.py, diff_diff/staggered.py, diff_diff/triple_diff.py, diff_diff/dml_did.py, diff_diff/lwdid.py, docs/methodology/REGISTRY.md] + notes: "Family-wide pscore_trim validation via the shared utils.validate_pscore_trim (promoted from the TripleDifference/DMLDiD inline copies, message text unchanged from theirs; the [M-142] shape precedent). Behavior deltas per consumer: ContinuousDiD - the headline change - tightens from 0.0 <= x < 0.5 to 0 < x < 0.5 (trim=0 disabled the np.clip overlap guard keeping the 1/(1-p) IPW/DR weights finite), closes the np.isfinite type hole (None/str/Decimal/Fraction: TypeError -> ValueError; a 1-element ndarray: silently ACCEPTED -> rejected), coerces numpy scalars to built-in float, and changes its message from 'Invalid pscore_trim: ...' to the shared wording. CallawaySantAnna's bare range checks (__init__ + the fit-path mutation re-check) gain the same type guard and coercion, with the range message unchanged. TripleDifference gains only the float coercion (its inline guard already matched). LWDiD changes message wording only (its isinstance guard already matched). DMLDiD is behavior-identical (its module-local copy WAS the promoted helper). No in-repo caller passed out-of-domain values. introduced_in 4.0: shipped post-3.9-cut, the locked ladder's next release (tests/test_naming_guard.py _NEXT_RELEASE); status done is terminal so the row gates nothing further. Deprecated StaggeredTripleDifference deliberately keeps its bare range check (construction-permissive dying class, [M-013]/[M-144] posture)." + - id: M-146 + kind: behavior + group: policy-summary-alpha + old: "diff_diff:CallawaySantAnnaResults.summary[alpha]" + new: null + introduced_in: "4.0" + deprecated_in: null + removed_in: null + status: done + phase: 5 + test_ref: tests/test_staggered.py + code_refs: [diff_diff/results_base.py, diff_diff/staggered_results.py, diff_diff/staggered_triple_diff_results.py, diff_diff/chaisemartin_dhaultfoeuille_results.py, diff_diff/imputation_results.py, diff_diff/efficient_did_results.py, diff_diff/two_stage_results.py, diff_diff/stacked_did_results.py, diff_diff/sun_abraham.py, docs/methodology/REGISTRY.md] + notes: "Staggered-family summary(alpha=)/print_summary(alpha=) tightening via the shared results_base._require_fit_alpha guard (the DMLDiDResults/EventStudyResults precedent): eight results classes (CallawaySantAnnaResults, StaggeredTripleDiffResults, ChaisemartinDHaultfoeuilleResults, ImputationDiDResults, EfficientDiDResults, TwoStageDiDResults, StackedDiDResults, SunAbrahamResults) previously did alpha = alpha or self.alpha and relabeled the confidence-interval header at the REQUESTED alpha while always printing the FIT-TIME stored intervals - silent coverage mislabeling on any non-fit alpha (bootstrap percentile intervals cannot be reconstructed from the SE at all). Behavior delta: accepted-and-mislabeled -> loud ValueError; alpha=0.0, previously swallowed by the falsy `or` idiom, now raises too. StaggeredTripleDiffResults is INCLUDED although its parent estimator is removed at 4.0 ([M-013]/[M-014]) - the mislabel is a live 3.x rendering bug, distinct from the estimator's frozen construction shape. The non-staggered summaries with the same idiom are tracked as a TODO.md audit row, not silently left behind. introduced_in 4.0 per the [M-144] rationale; status done is terminal." diff --git a/docs/v4-design.md b/docs/v4-design.md index d115bb431..510bde1b7 100644 --- a/docs/v4-design.md +++ b/docs/v4-design.md @@ -849,7 +849,7 @@ above; anything only one PR cares about stays in that PR's plan.** | 2: contract foundations | 3.9 | (a) results base + unified event-study representation [M-092] + to_dict completion + the Diagnostic marker base on the diagnostic result roster [M-091] (section 3.5); (b) `aggregate()` + fit(aggregate=) shims [M-020..M-027] [M-139] (M-020's shim already shipped; M-139 is the HAD workflow twin, a pre-cut amendment); (c) param renames [M-030..M-047] [M-084] [M-086..M-089] + their results-field mirrors [M-094] [M-095] (section 8 rule 9) + the public-function completeness sweep [M-097..M-113] (section 8 rule 10) + the dCDH results mirror [M-114] + the fourth `robust` site [M-115] + the 2(c)-ii missed-rename amendments [M-136..M-138] (LPDiD `level` value; the two post-dummy diagnostics params) + BaseEstimator mixin + ContinuousDiD covariates move; (d) alias introduction [M-062] (the Spillover introduction is cancelled [M-063]) + the alias-diet `__getattr__` warning shim [M-135] + wrapper deprecations [M-070..M-077] + the two inference-surface policies: `n_bootstrap` semantic unification [M-081] and the wild-cluster-bootstrap roster guard [M-096]; shipped insertions (all done): the aggregate contract [M-122], the ETWFE reference-period family [M-123] [M-124] [M-125], and the variance-consolidation program [M-126] [M-127] | | 3: merges | 3.9 | (a) TWFE event-study mode [M-010] + EventStudy warn [M-060] + the fit `time`->`post` rename [M-082] (gates: section 4.1's equivalence/divergence/pooled-parity test triple) (shipped: tests/test_v4_merge_mpd.py; consumer ports incl. HonestDiD/PreTrendsPower calendar routes); (b) TripleDifference facade [M-013] + the SDDD alias [M-064] (shipped: tests/test_v4_merge_ddd.py; the engine relocation into the private shared mixin, the keyword-only staggered fit params, and the pscore_trim tightening [M-142]. The fit-time aggregate=/balance_e= carve-out rows are scheduled REMOVALS and so are cited in the phase-5 cell, not here - their only lifecycle version is removed_in 4.0); (c) CiC method= [M-015] + its results-field mirror [M-143] (shipped: tests/test_v4_merge_cic.py; method= is keyword-only and lowercase-only, the QDiD CLASS is deprecated while the METHOD is not, and ChangesInChangesResults.estimator -> .method carries a dual-key to_dict() window through 3.9) | | 4: release + soak | 3.9 cut | Migration guide written (skeleton: section 10); maintainer cuts 3.9; maint/3.8 rule active | -| 5: enforcement | 4.0 | Removals [M-010..M-015, M-020..M-027, M-139, M-030, M-032..M-047 old names, M-060, M-061, M-064, M-070..M-077, M-084, M-086..M-089, M-001..M-003, M-117, M-118, M-119, M-120, M-140, M-141, M-143] + the alias diet [M-132]..[M-134] + the amendment's old names [M-094] [M-095] [M-097..M-115] [M-136..M-138] (incl. their consumer migrations and the `clean_control` serialized reporting key); M-031's old `time` name persists as the merged class's calendar column, so it is deliberately absent from the removal roster (its 4.0 enforcement is the M-085 behavior entry below); property window: [M-016] property-flips at 4.0 (removal at 5.0); storage flips [M-050..M-058]; default policies [M-004..M-006, M-128..M-131, M-080]; merged-class behavior enforcements [M-083] [M-085]; warning retirement [M-007]; fastpath go/no-go [M-008]; diagnostic-family docs/roster reorganization [M-090]; sentinel retirement [M-093]; the family-wide anticipation validation [M-144] (behavior tightening landing at 4.0 - shipped post-3.9-cut, terminal `done`, no removal/deprecation fields); docs/llms.txt/README refresh | +| 5: enforcement | 4.0 | Removals [M-010..M-015, M-020..M-027, M-139, M-030, M-032..M-047 old names, M-060, M-061, M-064, M-070..M-077, M-084, M-086..M-089, M-001..M-003, M-117, M-118, M-119, M-120, M-140, M-141, M-143] + the alias diet [M-132]..[M-134] + the amendment's old names [M-094] [M-095] [M-097..M-115] [M-136..M-138] (incl. their consumer migrations and the `clean_control` serialized reporting key); M-031's old `time` name persists as the merged class's calendar column, so it is deliberately absent from the removal roster (its 4.0 enforcement is the M-085 behavior entry below); property window: [M-016] property-flips at 4.0 (removal at 5.0); storage flips [M-050..M-058]; default policies [M-004..M-006, M-128..M-131, M-080]; merged-class behavior enforcements [M-083] [M-085]; warning retirement [M-007]; fastpath go/no-go [M-008]; diagnostic-family docs/roster reorganization [M-090]; sentinel retirement [M-093]; the family-wide anticipation validation [M-144], the family-wide pscore_trim validation [M-145], and the staggered-family summary-alpha guard [M-146] (behavior tightenings landing at 4.0 - shipped post-3.9-cut, terminal `done`, no removal/deprecation fields); docs/llms.txt/README refresh | | 6: front door | 4.1 | `event_study(data, outcome, unit, time, first_treat, estimator=...)` comparison entry point over the staggered family (sketch only; specified in its own plan) | Citation semantic for the table: a cell may cite a row whose current `phase` @@ -1111,7 +1111,8 @@ forever - a removed symbol resurrecting is a test failure. class/function rows and alias rows also assert `__all__` membership consistent with their status (stale `import *` entries fail). The shipped row ids are a - committed snapshot in the enforcement test (126 as of the family-wide + committed snapshot in the enforcement test (128 as of the DML + review-follow-ups pair; previously 126 as of the family-wide anticipation validation row: Phase 1 + the diagnostic-family amendment + the M-092/M-093 results-contract rows + the M-094..M-096 amendment rows + @@ -1119,7 +1120,7 @@ forever - a removed symbol resurrecting is a test failure. reference-period pair M-123/M-124 + M-125 + M-126 + M-127..M-131 + the alias-diet family M-132..M-135 + the 2(c)-ii amendments M-136..M-138 + M-139 + the DDD-merge rows M-140..M-142 + the CiC - results-field mirror M-143 + the anticipation policy row M-144; + results-field mirror M-143 + the anticipation policy row M-144 + the DML review-follow-ups pair M-145/M-146; the snapshot extends by a new id range in the same diff that appends rows): ids are never deleted or reused, and the test fails if any snapshot id disappears. diff --git a/tests/test_chaisemartin_dhaultfoeuille.py b/tests/test_chaisemartin_dhaultfoeuille.py index f08129284..7478cfa52 100644 --- a/tests/test_chaisemartin_dhaultfoeuille.py +++ b/tests/test_chaisemartin_dhaultfoeuille.py @@ -12396,3 +12396,29 @@ def test_heterogeneity_underidentified_nan_fills(self): assert np.isnan(h["t_stat"]) assert np.isnan(h["p_value"]) assert h["n_obs"] == 4 + + +@pytest.fixture(scope="module") +def alpha_fitted(): + data = generate_reversible_did_data(n_groups=40, n_periods=5, seed=1) + return ChaisemartinDHaultfoeuille().fit( + data, outcome="outcome", unit="group", time="period", treatment="treatment" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha): a non-fit alpha + raises instead of silently relabeling the confidence-interval header + over fit-time stored intervals; alpha=0.0 (previously swallowed by the + falsy `alpha or self.alpha` idiom) now raises too. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() diff --git a/tests/test_continuous_did.py b/tests/test_continuous_did.py index 509f09c89..c3e624c10 100644 --- a/tests/test_continuous_did.py +++ b/tests/test_continuous_did.py @@ -3,6 +3,8 @@ """ import warnings +from decimal import Decimal +from fractions import Fraction import numpy as np import pandas as pd @@ -1666,6 +1668,32 @@ def test_invalid_nuisance_params_raise(self): with pytest.raises(ValueError, match="epv_threshold"): ContinuousDiD(epv_threshold=0.0) + def test_pscore_trim_zero_rejected(self): + """trim=0 disables the np.clip overlap guard - now rejected, matching + the TripleDifference tightening (row M-142) via the shared helper.""" + with pytest.raises(ValueError, match=r"pscore_trim must be in \(0, 0.5\)"): + ContinuousDiD(pscore_trim=0) + + @pytest.mark.parametrize( + "bad", + [ + None, + "0.01", + True, + np.array([0.01]), + Decimal("0.01"), + Fraction(1, 100), + ], + ) + def test_pscore_trim_type_guard(self, bad): + """Non-real-scalar inputs raise ValueError, closing the old + np.isfinite(...) TypeError hole (and 1-element-array acceptance).""" + with pytest.raises(ValueError, match="pscore_trim must be"): + ContinuousDiD(pscore_trim=bad) + + def test_pscore_trim_numpy_float_coerced(self): + assert type(ContinuousDiD(pscore_trim=np.float32(0.01)).pscore_trim) is float + def test_covariate_metadata_on_results(self): data = _cov_data() est = ContinuousDiD( diff --git a/tests/test_dr_scores.py b/tests/test_dr_scores.py index 5094b3e9b..4ab912419 100644 --- a/tests/test_dr_scores.py +++ b/tests/test_dr_scores.py @@ -320,3 +320,24 @@ def test_augmented_validation(self): T_bad = T.copy() T_bad[0] = 3.0 chang_rcs_score_augmented(summand, D, T_bad, y, m2, ps, 1.0, 0.5, 0.5) + + def test_internal_with_slope_variant_matches_public_pair(self): + """The single-pass internal variant equals the two public calls exactly.""" + from diff_diff._dr_scores import ( + _chang_rcs_score_augmented_with_slope, + chang_rcs_lambda_slope, + chang_rcs_score, + chang_rcs_score_augmented, + ) + + y, D, T, m2, ps = self._inputs() + p_hat, lam_hat = 0.5, 0.45 + summand = chang_rcs_score(y, D, T, m2, ps, p_hat, lam_hat) + theta = float(np.mean(summand)) + psi_bar, g2_lambda = _chang_rcs_score_augmented_with_slope( + summand, D, T, y, m2, ps, theta, p_hat, lam_hat + ) + np.testing.assert_array_equal( + psi_bar, chang_rcs_score_augmented(summand, D, T, y, m2, ps, theta, p_hat, lam_hat) + ) + assert g2_lambda == chang_rcs_lambda_slope(y, D, T, m2, ps, p_hat, lam_hat) diff --git a/tests/test_efficient_did.py b/tests/test_efficient_did.py index 6c8510bc6..09bc61b74 100644 --- a/tests/test_efficient_did.py +++ b/tests/test_efficient_did.py @@ -3561,3 +3561,28 @@ def counting_cached(X, degree, cache): assert len(build_keys) == len(set(request_keys)) # ...and there was genuine redundancy for the cache to eliminate. assert len(request_keys) > len(build_keys) + + +@pytest.fixture(scope="module") +def alpha_fitted(): + return EfficientDiD(pt_assumption="all").fit( + _make_simple_panel(), "y", "unit", "time", "first_treat" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha): a non-fit alpha + raises instead of silently relabeling the confidence-interval header + over fit-time stored intervals; alpha=0.0 (previously swallowed by the + falsy `alpha or self.alpha` idiom) now raises too. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() diff --git a/tests/test_event_study_consumers.py b/tests/test_event_study_consumers.py index 477f5bf22..11e13ed1b 100644 --- a/tests/test_event_study_consumers.py +++ b/tests/test_event_study_consumers.py @@ -2147,3 +2147,134 @@ def test_pretrends_accepts_dml_container(self, dml_universal): es = dml_universal.aggregate("event_study") power = compute_pretrends_power(es, M=0.1) assert power is not None + + +class TestHonestRawRouteZeroSE: + """plot_honest_event_study's raw (non-container) routes mirror the + container _retained semantics: zero/non-finite-SE rows are excluded + (they carry no honest interval), the reference row is kept as a + normalization-only anchor, and there is deliberately NO -1 reference + fallback on marker-less surfaces.""" + + @staticmethod + def _honest(original, bounds=None): + from types import SimpleNamespace + + return SimpleNamespace( + original_results=original, + alpha=0.05, + event_study_bounds=bounds, + ci_lb=-1.0, + ci_ub=1.0, + M=0.5, + ) + + @staticmethod + def _ticks(ax): + return [t.get_text() for t in ax.get_xticklabels()] + + def test_cs_dict_route_filters_and_keeps_signature_reference(self): + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + nan = float("nan") + original = SimpleNamespace( + event_study_effects={ + -1: {"effect": 0.0, "se": nan, "n_groups": 0}, + 0: {"effect": 1.0, "se": 0.5, "n_groups": 3}, + 1: {"effect": 1.2, "se": 0.0, "n_groups": 3}, + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), show=False) + ticks = self._ticks(ax) + assert "-1" in ticks and "0" in ticks and "1" not in ticks + + with pytest.raises(ValueError, match="undefined inference"): + plot_honest_event_study(self._honest(original), periods=[-1, 0, 1], show=False) + + def test_mpd_route_filters_zero_se(self): + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + original = SimpleNamespace( + period_effects={ + 1: SimpleNamespace(effect=1.0, se=0.5), + 2: SimpleNamespace(effect=1.2, se=0.0), + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), show=False) + ticks = self._ticks(ax) + assert "1" in ticks and "2" not in ticks + + def test_markerless_surface_has_no_minus_one_fallback(self): + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + # A genuinely ESTIMATED zero-SE e=-1 row on a marker-less surface: + # no signature row, no reference_period attribute -> NO inferred + # reference, so the row is excluded rather than promoted to a + # hollow normalization anchor. + original = SimpleNamespace( + event_study_effects={ + -1: {"effect": 0.7, "se": 0.0, "n_groups": 3}, + 0: {"effect": 1.0, "se": 0.5, "n_groups": 3}, + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), show=False) + ticks = self._ticks(ax) + assert "-1" not in ticks and "0" in ticks + + def test_all_rows_undefined_raises_not_blank_figure(self): + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + original = SimpleNamespace( + event_study_effects={ + 0: {"effect": 1.0, "se": 0.0, "n_groups": 3}, + 1: {"effect": 1.2, "se": float("nan"), "n_groups": 3}, + } + ) + with pytest.raises(ValueError, match="No valid data to plot"): + plot_honest_event_study(self._honest(original), show=False) + + +class TestContainerExplicitReferenceZeroSE: + """plot_event_study on an EventStudyResults container with an explicit + reference_period= discards both override channels, so rows fall to the + reconstruction gate: a zero-SE non-reference row must not resurface as + a finite zero-width interval (round-5 review finding).""" + + def test_zero_se_row_nan_gated_after_explicit_normalization(self): + from diff_diff.visualization import plot_event_study + + surface = _tiny_container( + se=np.array([0.1, np.nan, 0.12, 0.0]), + t_stat=np.array([1.0, np.nan, 15.8, np.nan]), + p_value=np.array([0.3, np.nan, 0.0, np.nan]), + conf_int_lower=np.array([-0.1, np.nan, 1.66, np.nan]), + conf_int_upper=np.array([0.3, np.nan, 2.14, np.nan]), + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_event_study(surface, reference_period=-1, show=False) + # event_time 1 (the zero-SE row) sits at ordinal x position 3. + effect_at_1 = float(surface.att[3]) + for coll in ax.collections: + if not hasattr(coll, "get_segments"): + continue + for seg in coll.get_segments(): + if len(seg) == 2 and abs(float(seg[0][0]) - 3.0) < 1e-9: + lo_y, hi_y = float(seg[0][1]), float(seg[1][1]) + assert not ( + abs(lo_y - effect_at_1) < 1e-12 and abs(hi_y - effect_at_1) < 1e-12 + ), "zero-SE container row drew a zero-width CI after explicit normalization" diff --git a/tests/test_imputation.py b/tests/test_imputation.py index 2dbe319bc..c686c9563 100644 --- a/tests/test_imputation.py +++ b/tests/test_imputation.py @@ -3455,3 +3455,29 @@ def test_validation_and_transactional_set_params(self): est.set_params(df_convention="normal", nonexistent_param=1) assert est.get_params() == before assert ImputationDiD(df_convention="normal").get_params()["df_convention"] == "normal" + + +@pytest.fixture(scope="module") +def alpha_fitted(): + data = generate_test_data() + return ImputationDiD().fit( + data, outcome="outcome", unit="unit", time="time", first_treat="first_treat" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha): a non-fit alpha + raises instead of silently relabeling the confidence-interval header + over fit-time stored intervals; alpha=0.0 (previously swallowed by the + falsy `alpha or self.alpha` idiom) now raises too. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() diff --git a/tests/test_lwdid.py b/tests/test_lwdid.py index 4a8514d9b..7bef8a227 100644 --- a/tests/test_lwdid.py +++ b/tests/test_lwdid.py @@ -2828,6 +2828,14 @@ def test_seeded_bootstrap_invariant_to_n_jobs(self): assert r1.se == r2.se assert r1.att == r2.att + def test_pscore_trim_rejections_use_shared_message(self): + """LWDiD now validates via utils.validate_pscore_trim: same guard + behavior as before, but the shared message wording.""" + with pytest.raises(ValueError, match=r"pscore_trim must be in \(0, 0.5\)"): + LWDiD(rolling="demean", pscore_trim=0.6) + with pytest.raises(ValueError, match="pscore_trim must be a real number"): + LWDiD(rolling="demean", pscore_trim="0.01") + def test_pscore_trim_provenance(self): df = self._panel(x=lambda u: float(u % 4)) with warnings.catch_warnings(): diff --git a/tests/test_stacked_did.py b/tests/test_stacked_did.py index b74cbfddb..09180c4c9 100644 --- a/tests/test_stacked_did.py +++ b/tests/test_stacked_did.py @@ -2264,3 +2264,36 @@ def test_survey_zeroed_reference_cell_fails_closed(self): first_treat="first_treat", survey_design=SurveyDesign(weights="w"), ) + + +@pytest.fixture(scope="module") +def alpha_fitted(): + data = generate_staggered_data( + n_units=120, + n_periods=10, + cohort_periods=[4, 6], + never_treated_frac=0.3, + treatment_effect=5.0, + seed=42, + ) + return StackedDiD(kappa_pre=2, kappa_post=2).fit( + data, outcome="outcome", unit="unit", time="period", first_treat="first_treat" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha): a non-fit alpha + raises instead of silently relabeling the confidence-interval header + over fit-time stored intervals; alpha=0.0 (previously swallowed by the + falsy `alpha or self.alpha` idiom) now raises too. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() diff --git a/tests/test_staggered.py b/tests/test_staggered.py index 00a835841..42d58d499 100644 --- a/tests/test_staggered.py +++ b/tests/test_staggered.py @@ -3,6 +3,8 @@ """ import warnings +from decimal import Decimal +from fractions import Fraction import numpy as np import pandas as pd @@ -4237,6 +4239,56 @@ def test_pscore_trim_zero_raises(self): with pytest.raises(ValueError, match="pscore_trim must be in"): CallawaySantAnna(pscore_trim=0.0) + @pytest.mark.parametrize( + "bad", + [ + None, + "0.01", + True, + np.array([0.01]), + Decimal("0.01"), + Fraction(1, 100), + ], + ) + def test_pscore_trim_type_guard(self, bad): + """Non-real-scalar inputs raise ValueError (shared utils helper). + + The old bare `0 < x < 0.5` check raised TypeError on None/str, + ACCEPTED a 1-element array, and accepted Decimal/Fraction. + """ + with pytest.raises(ValueError, match="pscore_trim must be"): + CallawaySantAnna(pscore_trim=bad) + + def test_pscore_trim_numpy_float_coerced(self): + """The shared helper coerces to a builtin float (DMLDiD precedent).""" + assert type(CallawaySantAnna(pscore_trim=np.float32(0.01)).pscore_trim) is float + + def test_fit_revalidates_directly_mutated_pscore_trim(self): + """Direct attribute mutation is caught by the fit-time re-check. + + Uses a TYPE-guard value (1-element array) the OLD bare range check + silently accepted, so this test detects a missed migration of the + fit-time site - an out-of-range float would raise under either. + """ + np.random.seed(42) + n_units, n_periods = 30, 4 + units = np.repeat(np.arange(n_units), n_periods) + times = np.tile(np.arange(n_periods), n_units) + first_treat = np.where(units < 15, 2, 0) + data = pd.DataFrame( + { + "unit": units, + "time": times, + "first_treat": first_treat, + "outcome": np.random.normal(size=n_units * n_periods) + + 0.5 * ((first_treat > 0) & (times >= first_treat)), + } + ) + cs = CallawaySantAnna() + cs.pscore_trim = np.array([0.01]) + with pytest.raises(ValueError, match="pscore_trim"): + cs.fit(data, outcome="outcome", unit="unit", time="time", first_treat="first_treat") + def test_pscore_trim_in_results(self): """results.pscore_trim matches the estimator's setting after fit().""" np.random.seed(42) @@ -6180,3 +6232,33 @@ def test_survey_design_psu_wins_under_bootstrap(self): f"reference SE ({res_ref.overall_se}) — both bootstraps must " "draw at the same effective PSU level." ) + + +@pytest.fixture(scope="module") +def alpha_fitted(): + data = generate_staggered_data(n_units=40, n_periods=6) + return CallawaySantAnna().fit( + data, outcome="outcome", unit="unit", time="time", first_treat="first_treat" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha): a non-fit alpha + raises instead of silently relabeling the confidence-interval header + over fit-time stored intervals; alpha=0.0 (previously swallowed by the + falsy `alpha or self.alpha` idiom) now raises too. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() + + def test_print_summary_relays_the_guard(self, alpha_fitted): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.print_summary(alpha=0.10) diff --git a/tests/test_staggered_triple_diff.py b/tests/test_staggered_triple_diff.py index 12eab70ac..c95173d2a 100644 --- a/tests/test_staggered_triple_diff.py +++ b/tests/test_staggered_triple_diff.py @@ -729,3 +729,28 @@ def test_no_covariates_no_warning(self, simple_data): est.fit(simple_data, "outcome", "unit", "period", "first_treat", "eligibility") lstsq_warnings = [w for w in caught if "Rank-deficient X'WX" in str(w.message)] assert lstsq_warnings == [] + + +@pytest.fixture(scope="module") +def alpha_fitted(): + data = generate_staggered_ddd_data(n_units=300, treatment_effect=3.0, seed=42) + return StaggeredTripleDifference().fit( + data, "outcome", "unit", "period", "first_treat", "eligibility" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha) - applies to this + results class even though the parent estimator is deprecated: the + summary surface is live through 3.x. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() diff --git a/tests/test_sun_abraham.py b/tests/test_sun_abraham.py index 23df78c6f..c236a237b 100644 --- a/tests/test_sun_abraham.py +++ b/tests/test_sun_abraham.py @@ -2208,3 +2208,29 @@ def test_get_params_roundtrip_and_repeat_fit(self): r2 = clone.fit(data, **self._kw) assert r2.overall_p_value == p1 and r2.inference_df == d1 assert clone.df_convention == "cluster" + + +@pytest.fixture(scope="module") +def alpha_fitted(): + data = generate_staggered_data() + return SunAbraham().fit( + data, outcome="outcome", unit="unit", time="time", first_treat="first_treat" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha): a non-fit alpha + raises instead of silently relabeling the confidence-interval header + over fit-time stored intervals; alpha=0.0 (previously swallowed by the + falsy `alpha or self.alpha` idiom) now raises too. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() diff --git a/tests/test_two_stage.py b/tests/test_two_stage.py index 88b4634c0..ced3ff9f4 100644 --- a/tests/test_two_stage.py +++ b/tests/test_two_stage.py @@ -2763,3 +2763,29 @@ def test_group_only_aggregate_has_no_es_vcov(self): assert res.event_study_vcov is None assert res.event_study_vcov_index is None assert res.event_study_df is None + + +@pytest.fixture(scope="module") +def alpha_fitted(): + data = generate_test_data() + return TwoStageDiD().fit( + data, outcome="outcome", unit="unit", time="time", first_treat="first_treat" + ) + + +class TestSummaryAlphaContract: + """summary(alpha=...) never recomputes stored inference. + + Family-wide guard (results_base._require_fit_alpha): a non-fit alpha + raises instead of silently relabeling the confidence-interval header + over fit-time stored intervals; alpha=0.0 (previously swallowed by the + falsy `alpha or self.alpha` idiom) now raises too. + """ + + @pytest.mark.parametrize("bad_alpha", [0.10, 0.0]) + def test_summary_rejects_non_fit_alpha(self, alpha_fitted, bad_alpha): + with pytest.raises(ValueError, match="never recomputes"): + alpha_fitted.summary(alpha=bad_alpha) + + def test_summary_accepts_fit_alpha(self, alpha_fitted): + assert alpha_fitted.summary(alpha=alpha_fitted.alpha) == alpha_fitted.summary() diff --git a/tests/test_v4_matrix.py b/tests/test_v4_matrix.py index d75d3f268..b8d1a52d1 100644 --- a/tests/test_v4_matrix.py +++ b/tests/test_v4_matrix.py @@ -129,11 +129,13 @@ # spent/earmarked) = 121; + the phase-3(b) DDD merge rows (M-140/M-141 carry # fit-time aggregate=/balance_e= onto the surviving TripleDifference, M-142 the # pscore_trim tightening) = 124; + the CiC results-field rename (M-143) = 125; -# + the family-wide anticipation validation row (M-144) = 126. +# + the family-wide anticipation validation row (M-144) = 126; + the DML +# review-follow-ups pair (M-145 family-wide pscore_trim validation, M-146 +# staggered-family summary-alpha guard) = 128. # Ids are never reused and terminal rows are never deleted, so the ledger # only grows - raise the floor when rows are added; a lower parse count # means scanner/format drift or an illegal row deletion. -ROW_COUNT_FLOOR = 126 +ROW_COUNT_FLOOR = 128 # Committed snapshot of the shipped id set ("ids are never deleted or reused" # contract - a delete-one-add-one edit keeps the count above the floor but trips @@ -188,6 +190,7 @@ (140, 142), (143, 143), (144, 144), + (145, 146), ] EXPECTED_INITIAL_IDS = frozenset( f"M-{n:03d}" for lo, hi in _INITIAL_ID_RANGES for n in range(lo, hi + 1) @@ -586,14 +589,14 @@ def test_initial_ids_never_deleted(): """The shipped id set is immutable: ids are never deleted or reused (spec section 11). ROW_COUNT_FLOOR alone would let a delete-one-add-one edit pass; this snapshot cannot. - Extends as rows ship (126 as of the family-wide anticipation validation row: + Extends as rows ship (128 as of the DML review-follow-ups pair: Phase 1 + diagnostic-family + M-092/M-093 + M-094..M-096 + the M-097..M-115 public-function completeness sweep + M-117..M-120/M-122 + M-123/M-124 + M-125 + M-126 + M-127..M-131 + M-132..M-135 + - M-136..M-138 + M-139 + M-140..M-142 + M-143 + M-144).""" + M-136..M-138 + M-139 + M-140..M-142 + M-143 + M-144 + M-145/M-146).""" missing = sorted(EXPECTED_INITIAL_IDS - set(_ROW_IDS)) assert not missing, f"ledger rows deleted (ids are permanent): {missing}" - assert len(EXPECTED_INITIAL_IDS) == 126 + assert len(EXPECTED_INITIAL_IDS) == 128 def test_version_tuple_pads_to_three_components(): @@ -1057,17 +1060,18 @@ def _changes_at_4_0(row): Keyed on the two LIFECYCLE version fields only - a symbol removed at 4.0, or one whose deprecation warning starts firing at 4.0 (the ``field-flip`` family, removed - at 5.0). 108 of the 126 rows qualify. + at 5.0). 108 of the 128 rows qualify. - The 18 that do not, and why (this enumeration is the contract - a reader of the + The 20 that do not, and why (this enumeration is the contract - a reader of the guide must be able to trust that nothing 4.0-relevant was dropped): - 12 ``behavior`` rows with ``introduced_in: 3.9`` and no dep/rem: already shipped in 3.9, so there is no 4.0 action. They get their own guide section, not an appendix row. - - 1 ``behavior`` row with ``introduced_in: 4.0`` and no deprecation/removal - fields (``M-144``, the post-cut anticipation validation tightening): it lands - AT 4.0 but removes/deprecates nothing, so it appears in the guide's + - 3 ``behavior`` rows with ``introduced_in: 4.0`` and no deprecation/removal + fields (``M-144`` anticipation validation, ``M-145`` pscore_trim validation, + ``M-146`` summary-alpha guard - the post-cut validation tightenings): they land + AT 4.0 but remove/deprecate nothing, so they appear in the guide's "Remaining 4.0 changes" prose, not the ledger-derived appendix. - ``M-062``, ``M-063``: aliases, introduce-only / all lifecycle fields null. - ``M-031``, ``M-082``: ``deprecated_in: 3.9`` with ``removed_in: null``, because diff --git a/tests/test_v4_merge_ddd.py b/tests/test_v4_merge_ddd.py index 67e4dc64b..14c1cf6e5 100644 --- a/tests/test_v4_merge_ddd.py +++ b/tests/test_v4_merge_ddd.py @@ -1082,6 +1082,10 @@ def test_pscore_trim_multi_element_array_rejected(self): def test_pscore_trim_interior_accepted(self, good): assert TripleDifference(pscore_trim=good).pscore_trim == good + def test_pscore_trim_coerced_to_builtin_float(self): + """The shared utils.validate_pscore_trim helper coerces on store.""" + assert type(TripleDifference(pscore_trim=np.float32(0.02)).pscore_trim) is float + def test_set_params_pscore_trim_is_transactional(self): est = TripleDifference(pscore_trim=0.01) with pytest.raises(ValueError): diff --git a/tests/test_visualization_new.py b/tests/test_visualization_new.py index 201d79799..2f9552222 100644 --- a/tests/test_visualization_new.py +++ b/tests/test_visualization_new.py @@ -731,3 +731,79 @@ def test_show_weighted_avg_adds_shapes(self): # Should have vertical line shapes (weighted avg + TWFE + zero line) shapes = fig.layout.shapes assert len(shapes) >= 4 # 3 weighted avg + 1 TWFE + zero line + + +class TestPlotEventStudyZeroSE: + """Zero-SE rows draw no finite zero-width pointwise interval, while an + auto-inferred reference row keeps its degenerate constraint bar (the + plot_group_effects twin gate + the REGISTRY reference-retention + contract).""" + + @staticmethod + def _fake(ref_conf_int): + nan = float("nan") + + class _Fake: + anticipation = 0 + + f = _Fake() + f.event_study_effects = { + -1: { + "effect": 0.0, + "se": 0.0, + "t_stat": nan, + "p_value": nan, + "conf_int": ref_conf_int, + "n_obs": 0, + }, + 0: { + "effect": 1.5, + "se": 0.0, + "t_stat": nan, + "p_value": nan, + "conf_int": (nan, nan), + "n_obs": 8, + }, + 1: { + "effect": 1.0, + "se": 0.5, + "t_stat": 2.0, + "p_value": 0.045, + "conf_int": (0.02, 1.98), + "n_obs": 8, + }, + } + return f + + @staticmethod + def _yerr_segments(ax): + segs = [] + for coll in ax.collections: + if hasattr(coll, "get_segments"): + segs.extend(coll.get_segments()) + return segs + + @pytest.mark.parametrize( + "ref_conf_int", + [(0.0, 0.0), (float("nan"), float("nan"))], # Imputation vs StackedDiD shapes + ) + def test_zero_se_gate_and_reference_retention(self, ref_conf_int): + from diff_diff.visualization import plot_event_study + + ax = plot_event_study(self._fake(ref_conf_int), show=False) + # x positions are ordinal: -1 -> 0, 0 -> 1, 1 -> 2 + saw_reference_bar = False + for seg in self._yerr_segments(ax): + if len(seg) != 2: + continue + x, lo_y, hi_y = float(seg[0][0]), float(seg[0][1]), float(seg[1][1]) + if abs(x - 1.0) < 1e-9: # the zero-SE NON-reference row + assert not ( + abs(lo_y - 1.5) < 1e-12 and abs(hi_y - 1.5) < 1e-12 + ), "zero-SE non-reference row drawn with a finite zero-width CI" + if abs(x - 0.0) < 1e-9 and abs(lo_y) < 1e-12 and abs(hi_y) < 1e-12: + saw_reference_bar = True + assert saw_reference_bar, ( + "auto-inferred reference row lost its degenerate (0, 0) bar " + "(REGISTRY: retained for auto-inferred)" + ) diff --git a/tests/test_visualization_plotly.py b/tests/test_visualization_plotly.py index e84f0f473..46dab1d78 100644 --- a/tests/test_visualization_plotly.py +++ b/tests/test_visualization_plotly.py @@ -520,3 +520,33 @@ def test_show_grid_false(self): ) assert fig.layout.xaxis.showgrid is False assert fig.layout.yaxis.showgrid is False + + +class TestPlotEventStudyZeroSEPlotly: + """The plotly CI band omits zero-SE non-reference rows entirely (the + renderer filters NaN-CI rows via has_ci; the polygon interpolating + across the gap is the pre-existing NaN-SE convention).""" + + def test_zero_se_row_absent_from_ci_band(self): + from diff_diff.visualization import plot_event_study + + nan = float("nan") + + class _Fake: + anticipation = 0 + + res = _Fake() + res.event_study_effects = { + -1: {"effect": 0.0, "se": 0.0, "conf_int": (0.0, 0.0), "n_obs": 0}, + 0: {"effect": 1.5, "se": 0.0, "conf_int": (nan, nan), "n_obs": 8}, + 1: {"effect": 1.0, "se": 0.5, "conf_int": (0.02, 1.98), "n_obs": 8}, + } + fig = plot_event_study(res, show=False, backend="plotly") + band_traces = [t for t in fig.data if getattr(t, "fill", None) == "toself"] + assert band_traces, "CI band trace missing" + band_x = set() + for t in band_traces: + band_x.update(float(x) for x in t.x) + # ordinal x: -1 -> 0.0 (reference, retained), 0 -> 1.0 (gated), 1 -> 2.0 + assert 1.0 not in band_x, "zero-SE non-reference row still in the CI band" + assert 0.0 in band_x and 2.0 in band_x From d70113c183b09f6e84370a067642c5a257c05bef Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 13:06:15 -0400 Subject: [PATCH 2/8] fix: validate pscore_trim after float coercion (extended-precision underflow); reject reference-only honest surfaces (PR #795 review P1s) --- diff_diff/utils.py | 12 +++++++++-- diff_diff/visualization/_event_study.py | 10 +++++++++ tests/test_continuous_did.py | 28 +++++++++++++++++++++++++ tests/test_event_study_consumers.py | 20 ++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/diff_diff/utils.py b/diff_diff/utils.py index d93df96f4..a9e6dbdb6 100644 --- a/diff_diff/utils.py +++ b/diff_diff/utils.py @@ -523,9 +523,17 @@ def validate_pscore_trim(value: Any) -> float: f"pscore_trim must be a real number in (0, 0.5), got {value!r} " f"(type {type(value).__name__})" ) - if not np.isfinite(value) or not 0 < value < 0.5: + # Coerce BEFORE the range check: an extended-precision np.longdouble can + # be positive in its own precision yet underflow to 0.0 as binary64, + # which would silently disable the overlap clip; an out-of-range Python + # int would raise a raw TypeError/OverflowError from np.isfinite/float. + try: + coerced = float(value) + except OverflowError: + raise ValueError(f"pscore_trim must be in (0, 0.5), got {value}") from None + if not np.isfinite(coerced) or not 0 < coerced < 0.5: raise ValueError(f"pscore_trim must be in (0, 0.5), got {value}") - return float(value) + return coerced # The staggered-only TripleDifference constructor params that genuinely select diff --git a/diff_diff/visualization/_event_study.py b/diff_diff/visualization/_event_study.py index d3ef0ad96..de2f251b6 100644 --- a/diff_diff/visualization/_event_study.py +++ b/diff_diff/visualization/_event_study.py @@ -1094,6 +1094,16 @@ def _defined(p: Any) -> bool: if periods is None: retained = [p for p in sorted(effects_dict.keys()) if _defined(p) or p == reference_period] + # The reference row is a normalization anchor, not an estimate: a + # surface whose every ESTIMATED row has undefined inference must + # raise, not render an anchor-only figure (REGISTRY all-undefined + # rejection). + if not any(_defined(p) for p in retained): + raise ValueError( + "No valid data to plot: every estimated period on this " + "event-study surface has undefined inference (non-finite " + "or zero SE)." + ) else: _bad = [p for p in periods if p in se_dict and not _defined(p) and p != reference_period] if _bad: diff --git a/tests/test_continuous_did.py b/tests/test_continuous_did.py index c3e624c10..6f119766f 100644 --- a/tests/test_continuous_did.py +++ b/tests/test_continuous_did.py @@ -1694,6 +1694,34 @@ def test_pscore_trim_type_guard(self, bad): def test_pscore_trim_numpy_float_coerced(self): assert type(ContinuousDiD(pscore_trim=np.float32(0.01)).pscore_trim) is float + def test_pscore_trim_never_stores_zero_after_coercion(self): + """Coercion-underflow guard (CI review): an extended-precision + np.longdouble positive in its own precision can underflow to 0.0 as + binary64, which would silently disable the overlap clip. The helper + validates the COERCED value, so it either raises or returns a + strictly positive float - on every longdouble width.""" + from diff_diff.utils import validate_pscore_trim + + for x in ( + np.longdouble(np.finfo(np.longdouble).smallest_subnormal), + np.longdouble(np.finfo(np.longdouble).tiny), + ): + try: + r = validate_pscore_trim(x) + except ValueError: + continue # underflowed to 0.0 and was rejected - correct + assert type(r) is float and 0.0 < r < 0.5 + + def test_pscore_trim_huge_int_raises_valueerror(self): + """An out-of-float-range Python int raises the documented ValueError, + not a raw OverflowError/TypeError from float()/np.isfinite.""" + from diff_diff.utils import validate_pscore_trim + + with pytest.raises(ValueError, match=r"pscore_trim must be in \(0, 0.5\)"): + validate_pscore_trim(10**400) + with pytest.raises(ValueError, match=r"pscore_trim must be in \(0, 0.5\)"): + validate_pscore_trim(10**20) + def test_covariate_metadata_on_results(self): data = _cov_data() est = ContinuousDiD( diff --git a/tests/test_event_study_consumers.py b/tests/test_event_study_consumers.py index 11e13ed1b..996ae2dca 100644 --- a/tests/test_event_study_consumers.py +++ b/tests/test_event_study_consumers.py @@ -2233,6 +2233,26 @@ def test_markerless_surface_has_no_minus_one_fallback(self): ticks = self._ticks(ax) assert "-1" not in ticks and "0" in ticks + def test_reference_only_surface_raises_not_anchor_figure(self): + # CI review P1: a retained reference row must not satisfy the + # empty-result guard on its own - a surface whose every ESTIMATED + # row has undefined inference raises rather than rendering an + # anchor-only figure (REGISTRY all-undefined rejection). + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + nan = float("nan") + original = SimpleNamespace( + event_study_effects={ + -1: {"effect": 0.0, "se": nan, "n_groups": 0}, + 0: {"effect": 1.0, "se": 0.0, "n_groups": 3}, + 1: {"effect": 1.2, "se": nan, "n_groups": 3}, + } + ) + with pytest.raises(ValueError, match="No valid data to plot"): + plot_honest_event_study(self._honest(original), show=False) + def test_all_rows_undefined_raises_not_blank_figure(self): from types import SimpleNamespace From f691af9b069900ce7b8c77928a75bbe5b55dea05 Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 13:13:28 -0400 Subject: [PATCH 3/8] fix: reject sub-ulp pscore_trim whose upper clip bound rounds to 1.0 (PR #795 review P1) --- diff_diff/utils.py | 9 +++++++++ tests/test_continuous_did.py | 17 ++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/diff_diff/utils.py b/diff_diff/utils.py index a9e6dbdb6..c5c8fbf05 100644 --- a/diff_diff/utils.py +++ b/diff_diff/utils.py @@ -533,6 +533,15 @@ def validate_pscore_trim(value: Any) -> float: raise ValueError(f"pscore_trim must be in (0, 0.5), got {value}") from None if not np.isfinite(coerced) or not 0 < coerced < 0.5: raise ValueError(f"pscore_trim must be in (0, 0.5), got {value}") + # A trim below half an ulp of 1.0 makes the upper clip bound + # 1 - trim round to exactly 1.0 in binary64, so np.clip would retain + # pscore == 1 and 1/(1-p) weights could divide by zero - the same + # disabled-overlap-guard failure trim=0 is rejected for. + if 1.0 - coerced == 1.0: + raise ValueError( + f"pscore_trim must be in (0, 0.5) and large enough that " + f"1 - pscore_trim < 1 in float64, got {value}" + ) return coerced diff --git a/tests/test_continuous_did.py b/tests/test_continuous_did.py index 6f119766f..18341bbaf 100644 --- a/tests/test_continuous_did.py +++ b/tests/test_continuous_did.py @@ -1709,8 +1709,23 @@ def test_pscore_trim_never_stores_zero_after_coercion(self): try: r = validate_pscore_trim(x) except ValueError: - continue # underflowed to 0.0 and was rejected - correct + continue # underflowed / sub-ulp and was rejected - correct assert type(r) is float and 0.0 < r < 0.5 + # The derived upper clip bound must remain strictly below 1. + assert 1.0 - r < 1.0 + + def test_pscore_trim_sub_ulp_rejected(self): + """Binary64 cancellation guard (CI review): a positive trim below + half an ulp of 1.0 makes 1 - trim round to exactly 1.0, so np.clip + would retain pscore == 1 - reject it like trim=0.""" + from diff_diff.utils import validate_pscore_trim + + assert 1.0 - 1e-20 == 1.0 # the failure mode being guarded + for bad in (1e-20, 5e-17, 2.0**-54): + with pytest.raises(ValueError, match="pscore_trim must be in"): + validate_pscore_trim(bad) + r = validate_pscore_trim(2.0**-52) # representable: 1 - 2**-52 < 1 + assert 1.0 - r < 1.0 def test_pscore_trim_huge_int_raises_valueerror(self): """An out-of-float-range Python int raises the documented ValueError, From ead13e4b122b4a5a4fe8e46de08baba2646dea72 Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 13:21:20 -0400 Subject: [PATCH 4/8] fix: honest-plot defined-estimate guard applies to explicit period selections; document the pscore_trim representability constraint (PR #795 review) --- diff_diff/utils.py | 8 +++++--- diff_diff/visualization/_event_study.py | 23 ++++++++++------------- docs/v4-deprecations.yaml | 2 +- tests/test_event_study_consumers.py | 19 +++++++++++++++++++ 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/diff_diff/utils.py b/diff_diff/utils.py index c5c8fbf05..b2f5d0272 100644 --- a/diff_diff/utils.py +++ b/diff_diff/utils.py @@ -514,9 +514,11 @@ def validate_pscore_trim(value: Any) -> float: range check (validate_n_bootstrap shape): a bare ``0 < x < 0.5`` raises an incidental TypeError on None/str/complex, an ambiguous-truth error on a multi-element array, and ACCEPTS a - 1-element array. StaggeredTripleDifference deliberately keeps its - bare range check (construction-permissive dying class, ledger - M-013/M-144). + 1-element array. Accepted values must additionally satisfy + ``1 - trim < 1`` in binary64: a sub-ulp positive trim would round the + upper clip bound to exactly 1.0, disabling the guard like ``trim=0``. + StaggeredTripleDifference deliberately keeps its bare range check + (construction-permissive dying class, ledger M-013/M-144). """ if isinstance(value, bool) or not isinstance(value, (int, float, np.integer, np.floating)): raise ValueError( diff --git a/diff_diff/visualization/_event_study.py b/diff_diff/visualization/_event_study.py index de2f251b6..b1d8477ba 100644 --- a/diff_diff/visualization/_event_study.py +++ b/diff_diff/visualization/_event_study.py @@ -1094,16 +1094,6 @@ def _defined(p: Any) -> bool: if periods is None: retained = [p for p in sorted(effects_dict.keys()) if _defined(p) or p == reference_period] - # The reference row is a normalization anchor, not an estimate: a - # surface whose every ESTIMATED row has undefined inference must - # raise, not render an anchor-only figure (REGISTRY all-undefined - # rejection). - if not any(_defined(p) for p in retained): - raise ValueError( - "No valid data to plot: every estimated period on this " - "event-study surface has undefined inference (non-finite " - "or zero SE)." - ) else: _bad = [p for p in periods if p in se_dict and not _defined(p) and p != reference_period] if _bad: @@ -1114,10 +1104,17 @@ def _defined(p: Any) -> bool: "analysis and they carry no honest interval." ) retained = list(periods) - if not retained: + # The reference row is a normalization anchor, not an estimate: with no + # defined ESTIMATED period retained - an all-undefined surface, or an + # explicit periods=[reference] selection - raise rather than render a + # meaningless anchor-only figure (REGISTRY all-undefined rejection; + # applies to BOTH the implicit and explicit selection branches). + if not any(_defined(p) for p in retained): raise ValueError( - "No valid data to plot: every period on this event-study " - "surface has undefined inference (non-finite or zero SE)." + "No valid data to plot: no retained period on this " + "event-study surface carries defined inference (non-finite " + "or zero SE everywhere, or only the reference anchor was " + "selected)." ) return retained diff --git a/docs/v4-deprecations.yaml b/docs/v4-deprecations.yaml index 768d3c4a3..3fd61929a 100644 --- a/docs/v4-deprecations.yaml +++ b/docs/v4-deprecations.yaml @@ -1758,7 +1758,7 @@ rows: phase: 5 test_ref: tests/test_continuous_did.py code_refs: [diff_diff/utils.py, diff_diff/continuous_did.py, diff_diff/staggered.py, diff_diff/triple_diff.py, diff_diff/dml_did.py, diff_diff/lwdid.py, docs/methodology/REGISTRY.md] - notes: "Family-wide pscore_trim validation via the shared utils.validate_pscore_trim (promoted from the TripleDifference/DMLDiD inline copies, message text unchanged from theirs; the [M-142] shape precedent). Behavior deltas per consumer: ContinuousDiD - the headline change - tightens from 0.0 <= x < 0.5 to 0 < x < 0.5 (trim=0 disabled the np.clip overlap guard keeping the 1/(1-p) IPW/DR weights finite), closes the np.isfinite type hole (None/str/Decimal/Fraction: TypeError -> ValueError; a 1-element ndarray: silently ACCEPTED -> rejected), coerces numpy scalars to built-in float, and changes its message from 'Invalid pscore_trim: ...' to the shared wording. CallawaySantAnna's bare range checks (__init__ + the fit-path mutation re-check) gain the same type guard and coercion, with the range message unchanged. TripleDifference gains only the float coercion (its inline guard already matched). LWDiD changes message wording only (its isinstance guard already matched). DMLDiD is behavior-identical (its module-local copy WAS the promoted helper). No in-repo caller passed out-of-domain values. introduced_in 4.0: shipped post-3.9-cut, the locked ladder's next release (tests/test_naming_guard.py _NEXT_RELEASE); status done is terminal so the row gates nothing further. Deprecated StaggeredTripleDifference deliberately keeps its bare range check (construction-permissive dying class, [M-013]/[M-144] posture)." + notes: "Family-wide pscore_trim validation via the shared utils.validate_pscore_trim (promoted from the TripleDifference/DMLDiD inline copies, message text unchanged from theirs; the [M-142] shape precedent). Behavior deltas per consumer: ContinuousDiD - the headline change - tightens from 0.0 <= x < 0.5 to 0 < x < 0.5 (trim=0 disabled the np.clip overlap guard keeping the 1/(1-p) IPW/DR weights finite), closes the np.isfinite type hole (None/str/Decimal/Fraction: TypeError -> ValueError; a 1-element ndarray: silently ACCEPTED -> rejected), coerces numpy scalars to built-in float, and changes its message from 'Invalid pscore_trim: ...' to the shared wording. CallawaySantAnna's bare range checks (__init__ + the fit-path mutation re-check) gain the same type guard and coercion, with the range message unchanged. The helper validates the COERCED binary64 value and additionally requires 1 - trim < 1 in float64: an extended-precision input underflowing to 0.0, an out-of-float-range int, or a sub-ulp positive trim (whose upper clip bound 1 - trim would round to exactly 1.0) all raise the documented ValueError rather than silently disabling the overlap clip. TripleDifference gains only the float coercion (its inline guard already matched). LWDiD changes message wording only (its isinstance guard already matched). DMLDiD is behavior-identical (its module-local copy WAS the promoted helper). No in-repo caller passed out-of-domain values. introduced_in 4.0: shipped post-3.9-cut, the locked ladder's next release (tests/test_naming_guard.py _NEXT_RELEASE); status done is terminal so the row gates nothing further. Deprecated StaggeredTripleDifference deliberately keeps its bare range check (construction-permissive dying class, [M-013]/[M-144] posture)." - id: M-146 kind: behavior group: policy-summary-alpha diff --git a/tests/test_event_study_consumers.py b/tests/test_event_study_consumers.py index 996ae2dca..3d79286d4 100644 --- a/tests/test_event_study_consumers.py +++ b/tests/test_event_study_consumers.py @@ -2253,6 +2253,25 @@ def test_reference_only_surface_raises_not_anchor_figure(self): with pytest.raises(ValueError, match="No valid data to plot"): plot_honest_event_study(self._honest(original), show=False) + def test_explicit_reference_only_selection_raises(self): + # CI review round-3 P1: periods=[reference] on an all-undefined + # surface must not bypass the defined-estimate guard via the + # explicit branch. + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + nan = float("nan") + original = SimpleNamespace( + event_study_effects={ + -1: {"effect": 0.0, "se": nan, "n_groups": 0}, + 0: {"effect": 1.0, "se": 0.0, "n_groups": 3}, + 1: {"effect": 1.2, "se": nan, "n_groups": 3}, + } + ) + with pytest.raises(ValueError, match="No valid data to plot"): + plot_honest_event_study(self._honest(original), periods=[-1], show=False) + def test_all_rows_undefined_raises_not_blank_figure(self): from types import SimpleNamespace From 944f472b32634769b671aa9be8f3ddd987af463f Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 13:30:38 -0400 Subject: [PATCH 5/8] docs: state the float64 representability condition in pscore_trim parameter docstrings (PR #795 review P2) --- diff_diff/continuous_did.py | 4 +++- diff_diff/dml_did.py | 4 +++- diff_diff/lwdid.py | 4 +++- diff_diff/staggered.py | 4 +++- diff_diff/triple_diff.py | 4 +++- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/diff_diff/continuous_did.py b/diff_diff/continuous_did.py index 83ba436cf..dd1ce123d 100644 --- a/diff_diff/continuous_did.py +++ b/diff_diff/continuous_did.py @@ -223,7 +223,9 @@ class ContinuousDiD(_ContinuousDiDAggregationMixin, BaseEstimator): ``overall_att`` / ATT(d) level and in its doubly-robust standard errors. pscore_trim : float, default=0.01 Propensity-score trimming bound for the ``dr`` path (scores clipped to - ``[pscore_trim, 1 - pscore_trim]``). Must be in ``(0, 0.5)``. + ``[pscore_trim, 1 - pscore_trim]``). Must be in ``(0, 0.5)`` and + large enough that ``1 - pscore_trim < 1`` in float64 (a sub-ulp + trim would disable the upper clip). epv_threshold : float, default=10.0 Events-per-variable threshold for the ``dr`` propensity logit diagnostics. pscore_fallback : str, default="error" diff --git a/diff_diff/dml_did.py b/diff_diff/dml_did.py index ec9b3c7b9..0403b9917 100644 --- a/diff_diff/dml_did.py +++ b/diff_diff/dml_did.py @@ -279,7 +279,9 @@ class DMLDiD(CallawaySantAnnaBootstrapMixin, CallawaySantAnnaAggregationMixin, B pscore_trim : float, default 0.01 Propensity clip bound: fitted propensities are clipped to ``[pscore_trim, 1 - pscore_trim]`` after the extremeness warning - (clip, never drop; Chang's paper gives no trimming rule). + (clip, never drop; Chang's paper gives no trimming rule). Must be in + ``(0, 0.5)`` and large enough that ``1 - pscore_trim < 1`` in + float64 (a sub-ulp trim would disable the upper clip). panel : bool, default True ``True`` estimates Chang's Case 1 (repeated outcomes) on panel data (one row per unit-period, cell score on outcome CHANGES). ``False`` diff --git a/diff_diff/lwdid.py b/diff_diff/lwdid.py index f1bbde0a3..1e8168104 100644 --- a/diff_diff/lwdid.py +++ b/diff_diff/lwdid.py @@ -461,7 +461,9 @@ class LWDiD(BaseEstimator): Random seed for bootstrap inference. pscore_trim : float, default 0.01 Propensity score trimming threshold. Scores below this value - or above (1 - pscore_trim) are clipped. Used by IPW/DR/PSM. + or above (1 - pscore_trim) are clipped. Used by IPW/DR/PSM. Must be in + ``(0, 0.5)`` and large enough that ``1 - pscore_trim < 1`` in + float64 (a sub-ulp trim would disable the upper clip). n_neighbors : int, default 1 Number of nearest neighbors for PSM matching. caliper : float or None, default None diff --git a/diff_diff/staggered.py b/diff_diff/staggered.py index a859c1b2f..c67176a45 100644 --- a/diff_diff/staggered.py +++ b/diff_diff/staggered.py @@ -450,7 +450,9 @@ class CallawaySantAnna( pscore_trim : float, default=0.01 Trimming bound for propensity scores. Scores are clipped to ``[pscore_trim, 1 - pscore_trim]`` before weight computation - in IPW and DR estimation. Must be in ``(0, 0.5)``. + in IPW and DR estimation. Must be in ``(0, 0.5)`` and large + enough that ``1 - pscore_trim < 1`` in float64 (a sub-ulp trim + would disable the upper clip). panel : bool, default=True Whether the data is a balanced/unbalanced panel (units observed across multiple time periods). Set to ``False`` for stationary diff --git a/diff_diff/triple_diff.py b/diff_diff/triple_diff.py index a43c6c962..0ee82c674 100644 --- a/diff_diff/triple_diff.py +++ b/diff_diff/triple_diff.py @@ -444,7 +444,9 @@ class TripleDifference( Significance level for confidence intervals. pscore_trim : float, default=0.01 Trimming threshold for propensity scores. Scores below this value - or above (1 - pscore_trim) are clipped to avoid extreme weights. + or above (1 - pscore_trim) are clipped to avoid extreme weights. Must be in + ``(0, 0.5)`` and large enough that ``1 - pscore_trim < 1`` in + float64 (a sub-ulp trim would disable the upper clip). rank_deficient_action : str, default="warn" Action when design matrix is rank-deficient (linearly dependent columns): From 8117d82673cb3ee1906fefdb04db633401bb681c Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 14:29:59 -0400 Subject: [PATCH 6/8] test: guard new event-study plot tests with the matplotlib importorskip fixture (CI no-matplotlib legs) --- tests/test_event_study_consumers.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_event_study_consumers.py b/tests/test_event_study_consumers.py index 3d79286d4..d02efd955 100644 --- a/tests/test_event_study_consumers.py +++ b/tests/test_event_study_consumers.py @@ -2156,6 +2156,15 @@ class TestHonestRawRouteZeroSE: normalization-only anchor, and there is deliberately NO -1 reference fallback on marker-less surfaces.""" + @pytest.fixture(autouse=True) + def _agg_backend(self): + matplotlib = pytest.importorskip("matplotlib") + matplotlib.use("Agg") + yield + import matplotlib.pyplot as plt + + plt.close("all") + @staticmethod def _honest(original, bounds=None): from types import SimpleNamespace @@ -2293,6 +2302,15 @@ class TestContainerExplicitReferenceZeroSE: reconstruction gate: a zero-SE non-reference row must not resurface as a finite zero-width interval (round-5 review finding).""" + @pytest.fixture(autouse=True) + def _agg_backend(self): + matplotlib = pytest.importorskip("matplotlib") + matplotlib.use("Agg") + yield + import matplotlib.pyplot as plt + + plt.close("all") + def test_zero_se_row_nan_gated_after_explicit_normalization(self): from diff_diff.visualization import plot_event_study From 69cfa6a047035251bdfd000c82ca98ab530f92c3 Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 14:40:46 -0400 Subject: [PATCH 7/8] fix(viz): require the anchor signature before the honest raw-route reference exemption (PR #795 review P0) An explicit reference_period= label pointing at an ESTIMATED zero-SE row promoted it past _honest_raw_route_periods' undefined-inference gate, drawing a zero-width original CI at a nonzero effect and suppressing its honest interval. The exemption now requires effect exactly 0.0 (every producer's true normalization row), mirroring plot_event_study's reference carve-out conjunct. Regression tests cover both raw routes, default and explicit periods selections, and the legitimate explicit-anchor case; REGISTRY Note updated. --- diff_diff/visualization/_event_study.py | 18 +++++-- docs/methodology/REGISTRY.md | 5 +- tests/test_event_study_consumers.py | 69 +++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/diff_diff/visualization/_event_study.py b/diff_diff/visualization/_event_study.py index b1d8477ba..5c2c977da 100644 --- a/diff_diff/visualization/_event_study.py +++ b/diff_diff/visualization/_event_study.py @@ -1085,17 +1085,29 @@ def _honest_raw_route_periods( and would otherwise be painted with a zero-width original CI plus the aggregate honest interval (or KeyError on per-period bounds), so they are excluded up front; the reference row is kept as a - normalization-only anchor. + normalization-only anchor - but only when it carries the anchor + signature (effect exactly 0.0), so a caller-supplied + ``reference_period`` label cannot promote an estimated zero-SE row + past the gate. """ def _defined(p: Any) -> bool: s = se_dict.get(p, float("nan")) return bool(np.isfinite(s) and float(s) > 0) + def _is_anchor(p: Any) -> bool: + # The reference exemption requires the anchor SIGNATURE (effect + # exactly 0.0 - every producer's true normalization row), not just + # the caller's label: an explicit reference_period= pointing at an + # ESTIMATED zero-SE row must not promote it past the gate (it + # would draw a zero-width CI at a nonzero effect and suppress its + # honest interval). + return p == reference_period and effects_dict.get(p) == 0.0 + if periods is None: - retained = [p for p in sorted(effects_dict.keys()) if _defined(p) or p == reference_period] + retained = [p for p in sorted(effects_dict.keys()) if _defined(p) or _is_anchor(p)] else: - _bad = [p for p in periods if p in se_dict and not _defined(p) and p != reference_period] + _bad = [p for p in periods if p in se_dict and not _defined(p) and not _is_anchor(p)] if _bad: raise ValueError( f"Requested periods {_bad} have undefined inference " diff --git a/docs/methodology/REGISTRY.md b/docs/methodology/REGISTRY.md index 42bf8c2bb..4c848994f 100644 --- a/docs/methodology/REGISTRY.md +++ b/docs/methodology/REGISTRY.md @@ -5910,7 +5910,10 @@ should be a deliberate user choice. rows are excluded up front (explicitly requesting one raises), the reference row - now auto-inferred on raw routes from a `reference_period` attribute or HonestDiD's own constraint signature, never a bare `-1` fallback - is kept as a normalization anchor, - and an all-undefined surface raises instead of rendering a blank figure. + and an all-undefined surface raises instead of rendering a blank figure. The reference + exemption requires the anchor signature (`effect` exactly 0.0), so an explicit + `reference_period=` label pointing at an estimated zero-SE row cannot promote it past + the gate (it is excluded on implicit selection and raises when explicitly requested). **Reference implementation(s):** - R: `fixest::coefplot()` with reference category shown at 0 with no CI diff --git a/tests/test_event_study_consumers.py b/tests/test_event_study_consumers.py index d02efd955..e9ad04e5c 100644 --- a/tests/test_event_study_consumers.py +++ b/tests/test_event_study_consumers.py @@ -2281,6 +2281,75 @@ def test_explicit_reference_only_selection_raises(self): with pytest.raises(ValueError, match="No valid data to plot"): plot_honest_event_study(self._honest(original), periods=[-1], show=False) + def test_explicit_reference_label_cannot_promote_estimated_zero_se_row(self): + # CI review round-5 P0: reference_period= is a caller label, not a + # verified anchor - pointing it at an ESTIMATED zero-SE row (effect + # != 0.0) must not retain the row (which would draw a zero-width CI + # at a nonzero effect and suppress its honest interval). The + # exemption requires the anchor signature: effect exactly 0.0. + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + original = SimpleNamespace( + event_study_effects={ + 0: {"effect": 1.0, "se": 0.5, "n_groups": 3}, + 1: {"effect": 1.2, "se": 0.0, "n_groups": 3}, + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), reference_period=1, show=False) + ticks = self._ticks(ax) + assert "0" in ticks and "1" not in ticks + + with pytest.raises(ValueError, match="undefined inference"): + plot_honest_event_study( + self._honest(original), reference_period=1, periods=[0, 1], show=False + ) + + def test_explicit_reference_label_mpd_route_zero_se_row_excluded(self): + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + original = SimpleNamespace( + period_effects={ + 1: SimpleNamespace(effect=1.0, se=0.5), + 2: SimpleNamespace(effect=1.2, se=0.0), + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), reference_period=2, show=False) + ticks = self._ticks(ax) + assert "1" in ticks and "2" not in ticks + + with pytest.raises(ValueError, match="undefined inference"): + plot_honest_event_study( + self._honest(original), reference_period=2, periods=[1, 2], show=False + ) + + def test_explicit_reference_with_anchor_signature_still_retained(self): + # The tightened exemption must not break the legitimate case: an + # explicit reference_period naming a true anchor row (effect 0.0). + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + nan = float("nan") + original = SimpleNamespace( + event_study_effects={ + -1: {"effect": 0.0, "se": nan, "n_groups": 0}, + 0: {"effect": 1.0, "se": 0.5, "n_groups": 3}, + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), reference_period=-1, show=False) + ticks = self._ticks(ax) + assert "-1" in ticks and "0" in ticks + def test_all_rows_undefined_raises_not_blank_figure(self): from types import SimpleNamespace From da2258892414132be16e30ad1526966d3ac61f64 Mon Sep 17 00:00:00 2001 From: igerber Date: Thu, 27 Aug 2026 14:55:04 -0400 Subject: [PATCH 8/8] fix(viz): verify anchor provenance, not the caller's reference label, on honest plot routes (PR #795 review P0) effect == 0.0 alone still let an ESTIMATED effect-0/se-0 row with a positive group count be promoted to a reference anchor by an explicit reference_period= label. _honest_raw_route_periods now takes the VERIFIED anchor resolved per route: dict surfaces require the producers' full constraint signature (zero n_groups/n_obs count, effect exactly 0.0, no defined SE - NaN or 0.0 markers), MPD surfaces verify only against the result's own reference_period metadata, and the container route only against is_reference-marked rows. Honest-bound suppression is keyed on the same verified anchor, so a mislabeled estimated row keeps its computed honest interval. Regression tests cover both raw routes, implicit and explicit selection; REGISTRY Note updated. --- diff_diff/visualization/_event_study.py | 67 ++++++++++++++++--------- docs/methodology/REGISTRY.md | 11 ++-- tests/test_event_study_consumers.py | 53 +++++++++++++++++++ 3 files changed, 105 insertions(+), 26 deletions(-) diff --git a/diff_diff/visualization/_event_study.py b/diff_diff/visualization/_event_study.py index 5c2c977da..e18b48019 100644 --- a/diff_diff/visualization/_event_study.py +++ b/diff_diff/visualization/_event_study.py @@ -1076,7 +1076,7 @@ def _honest_raw_route_periods( periods: Optional[List[Any]], effects_dict: Dict[Any, float], se_dict: Dict[Any, float], - reference_period: Optional[Any], + anchor_period: Optional[Any], ) -> List[Any]: """Period roster for the raw (non-container) honest-plot routes. @@ -1085,10 +1085,11 @@ def _honest_raw_route_periods( and would otherwise be painted with a zero-width original CI plus the aggregate honest interval (or KeyError on per-period bounds), so they are excluded up front; the reference row is kept as a - normalization-only anchor - but only when it carries the anchor - signature (effect exactly 0.0), so a caller-supplied - ``reference_period`` label cannot promote an estimated zero-SE row - past the gate. + normalization-only anchor. ``anchor_period`` is the VERIFIED anchor + (result metadata or the producer's constraint-row signature, resolved + by the caller) - a bare ``reference_period=`` label is never passed + through, so it cannot promote an estimated zero-SE row past the gate, + even one with effect exactly 0.0. """ def _defined(p: Any) -> bool: @@ -1096,13 +1097,7 @@ def _defined(p: Any) -> bool: return bool(np.isfinite(s) and float(s) > 0) def _is_anchor(p: Any) -> bool: - # The reference exemption requires the anchor SIGNATURE (effect - # exactly 0.0 - every producer's true normalization row), not just - # the caller's label: an explicit reference_period= pointing at an - # ESTIMATED zero-SE row must not promote it past the gate (it - # would draw a zero-width CI at a nonzero effect and suppress its - # honest interval). - return p == reference_period and effects_dict.get(p) == 0.0 + return anchor_period is not None and p == anchor_period if periods is None: retained = [p for p in sorted(effects_dict.keys()) if _defined(p) or _is_anchor(p)] @@ -1247,6 +1242,9 @@ def plot_honest_event_study( _ref_labels = [keys[i] for i, r in enumerate(original_results.is_reference) if r] if reference_period is None and len(_ref_labels) == 1: reference_period = _ref_labels[0] + # Verified anchor: the container's own is_reference metadata, not + # the caller's label. + _anchor_period = reference_period if reference_period in _ref_labels else None if periods is None: periods = sorted(effects_dict.keys()) else: @@ -1265,7 +1263,17 @@ def plot_honest_event_study( se_dict = {p: pe.se for p, pe in original_results.period_effects.items()} if reference_period is None: reference_period = getattr(original_results, "reference_period", None) - periods = _honest_raw_route_periods(periods, effects_dict, se_dict, reference_period) + # Verified anchor: only the result's own reference_period metadata + # - a caller label that does not match it is never an anchor on + # this route (period_effects rows carry no constraint signature to + # verify against). + _anchor_period = ( + reference_period + if reference_period is not None + and reference_period == getattr(original_results, "reference_period", None) + else None + ) + periods = _honest_raw_route_periods(periods, effects_dict, se_dict, _anchor_period) elif hasattr(original_results, "event_study_effects"): # CallawaySantAnnaResults (fit-time dict surface) effects_dict = { @@ -1288,7 +1296,22 @@ def plot_honest_event_study( ): reference_period = t break - periods = _honest_raw_route_periods(periods, effects_dict, se_dict, reference_period) + # Verified anchor: the resolved reference (attribute, signature + # match, or caller label) counts only if its row carries the + # producers' constraint signature - zero group/obs count, effect + # exactly 0.0, and no defined SE (NaN for CS, 0.0 for the + # Imputation/TwoStage/Stacked-style markers). An estimated row + # (positive count) is never an anchor, even at effect 0, se 0. + _anchor_period = None + _ref_data = original_results.event_study_effects.get(reference_period) + if ( + _ref_data is not None + and _ref_data.get("n_groups", _ref_data.get("n_obs", 1)) == 0 + and _ref_data["effect"] == 0.0 + and not (np.isfinite(_ref_data["se"]) and float(_ref_data["se"]) > 0) + ): + _anchor_period = reference_period + periods = _honest_raw_route_periods(periods, effects_dict, se_dict, _anchor_period) else: raise TypeError("Cannot extract event study data from original_results") @@ -1320,7 +1343,9 @@ def plot_honest_event_study( original_ci_lower = [effects_dict[p] - z * se_dict[p] for p in periods] original_ci_upper = [effects_dict[p] + z * se_dict[p] for p in periods] - # Get honest bounds if available for each period + # Get honest bounds if available for each period. Suppression is keyed + # on the VERIFIED anchor, not the caller's reference_period label - a + # mislabeled estimated row keeps its computed honest interval. _nan_bounds = {"ci_lb": np.nan, "ci_ub": np.nan} if honest_results.event_study_bounds: # The reference row is a normalization constraint with no honest @@ -1329,7 +1354,7 @@ def plot_honest_event_study( honest_ci_lower = [ ( honest_results.event_study_bounds.get(p, _nan_bounds) - if p == reference_period + if p == _anchor_period else honest_results.event_study_bounds[p] )["ci_lb"] for p in periods @@ -1337,7 +1362,7 @@ def plot_honest_event_study( honest_ci_upper = [ ( honest_results.event_study_bounds.get(p, _nan_bounds) - if p == reference_period + if p == _anchor_period else honest_results.event_study_bounds[p] )["ci_ub"] for p in periods @@ -1345,12 +1370,8 @@ def plot_honest_event_study( else: # Scalar bounds apply to every ESTIMATED period; the reference is # a constraint, never painted with the aggregate honest interval. - honest_ci_lower = [ - np.nan if p == reference_period else honest_results.ci_lb for p in periods - ] - honest_ci_upper = [ - np.nan if p == reference_period else honest_results.ci_ub for p in periods - ] + honest_ci_lower = [np.nan if p == _anchor_period else honest_results.ci_lb for p in periods] + honest_ci_upper = [np.nan if p == _anchor_period else honest_results.ci_ub for p in periods] if backend == "plotly": return _render_honest_event_study_plotly( diff --git a/docs/methodology/REGISTRY.md b/docs/methodology/REGISTRY.md index 4c848994f..fb7deaab1 100644 --- a/docs/methodology/REGISTRY.md +++ b/docs/methodology/REGISTRY.md @@ -5911,9 +5911,14 @@ should be a deliberate user choice. auto-inferred on raw routes from a `reference_period` attribute or HonestDiD's own constraint signature, never a bare `-1` fallback - is kept as a normalization anchor, and an all-undefined surface raises instead of rendering a blank figure. The reference - exemption requires the anchor signature (`effect` exactly 0.0), so an explicit - `reference_period=` label pointing at an estimated zero-SE row cannot promote it past - the gate (it is excluded on implicit selection and raises when explicitly requested). + exemption (and honest-bound suppression) applies only to a VERIFIED anchor, never a bare + `reference_period=` label: on dict surfaces the labeled row must carry the producers' + full constraint signature (zero `n_groups`/`n_obs` count, `effect` exactly 0.0, no + defined SE - NaN for CS, 0.0 for the Imputation/TwoStage/Stacked-style markers); on MPD + surfaces only the result's own `reference_period` metadata verifies; on the container + route only `is_reference`-marked rows do. An estimated zero-SE row - even one with + effect exactly 0.0 but a positive count - cannot be promoted past the gate (it is + excluded on implicit selection and raises when explicitly requested). **Reference implementation(s):** - R: `fixest::coefplot()` with reference category shown at 0 with no CI diff --git a/tests/test_event_study_consumers.py b/tests/test_event_study_consumers.py index e9ad04e5c..15f9fcb83 100644 --- a/tests/test_event_study_consumers.py +++ b/tests/test_event_study_consumers.py @@ -2330,6 +2330,59 @@ def test_explicit_reference_label_mpd_route_zero_se_row_excluded(self): self._honest(original), reference_period=2, periods=[1, 2], show=False ) + def test_explicit_reference_label_zero_effect_estimated_row_not_promoted(self): + # CI review round-6 P0: effect == 0.0 alone is not the anchor + # signature - an ESTIMATED row with effect exactly 0.0, se 0.0 and + # POSITIVE n_groups labeled via reference_period= must still be + # excluded (implicit) / rejected (explicit periods=), not promoted + # to a constraint with a [0, 0] CI and a suppressed honest + # interval. Anchor verification requires zero count. + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + original = SimpleNamespace( + event_study_effects={ + 0: {"effect": 1.0, "se": 0.5, "n_groups": 3}, + 1: {"effect": 0.0, "se": 0.0, "n_groups": 3}, + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), reference_period=1, show=False) + ticks = self._ticks(ax) + assert "0" in ticks and "1" not in ticks + + with pytest.raises(ValueError, match="undefined inference"): + plot_honest_event_study( + self._honest(original), reference_period=1, periods=[0, 1], show=False + ) + + def test_explicit_reference_label_zero_effect_mpd_row_not_promoted(self): + # MPD twin of the round-6 case: period_effects rows carry no + # constraint signature, so a caller label never verifies unless it + # matches the result's own reference_period metadata. + from types import SimpleNamespace + + from diff_diff.visualization import plot_honest_event_study + + original = SimpleNamespace( + period_effects={ + 1: SimpleNamespace(effect=1.0, se=0.5), + 2: SimpleNamespace(effect=0.0, se=0.0), + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + ax = plot_honest_event_study(self._honest(original), reference_period=2, show=False) + ticks = self._ticks(ax) + assert "1" in ticks and "2" not in ticks + + with pytest.raises(ValueError, match="undefined inference"): + plot_honest_event_study( + self._honest(original), reference_period=2, periods=[1, 2], show=False + ) + def test_explicit_reference_with_anchor_signature_still_retained(self): # The tightened exemption must not break the legitimate case: an # explicit reference_period naming a true anchor row (effect 0.0).