diff --git a/CHANGELOG.md b/CHANGELOG.md index 0033bb57..018af339 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 f0840b8e..25c394b7 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 5c78c9a3..69764c3f 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 043ce132..806e9a22 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 ff1712c4..74299e85 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 1e4b6390..dd1ce123 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,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]``). + ``[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" @@ -339,10 +346,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 a0de7a09..0403b991 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}") @@ -291,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`` @@ -399,7 +389,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 +1410,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 36ea67d9..82df5d08 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 3881b827..7b4f4670 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 14bc6efd..1e816810 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") @@ -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 @@ -626,13 +628,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 acbc7248..0b0962ef 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 89c81006..30073c75 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 2dfac0cb..c67176a4 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: @@ -449,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 @@ -612,8 +615,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 +1970,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 4363cce1..3e8fa710 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 f2942cbd..22c3e6be 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 7e7ff5b9..37409a9b 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 67e48f2b..0ee82c67 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: @@ -443,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): @@ -649,27 +652,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 39c24d7f..dd455417 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 42c53f0c..b2f5d027 100644 --- a/diff_diff/utils.py +++ b/diff_diff/utils.py @@ -503,6 +503,50 @@ 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. 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( + f"pscore_trim must be a real number in (0, 0.5), got {value!r} " + f"(type {type(value).__name__})" + ) + # 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}") + # 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 + + # 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 a59da41b..e18b4801 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,60 @@ def _extract_plot_data( ) +def _honest_raw_route_periods( + periods: Optional[List[Any]], + effects_dict: Dict[Any, float], + se_dict: Dict[Any, float], + anchor_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. ``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: + s = se_dict.get(p, float("nan")) + return bool(np.isfinite(s) and float(s) > 0) + + def _is_anchor(p: Any) -> bool: + 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)] + else: + _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 " + "(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) + # 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: 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 + + def plot_honest_event_study( honest_results: "HonestDiDResults", *, @@ -1090,9 +1155,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 @@ -1172,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: @@ -1188,16 +1261,57 @@ 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) + # 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 + # 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 + # 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") @@ -1229,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 @@ -1238,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 @@ -1246,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 @@ -1254,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 bb9483fc..fb7deaab 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,36 @@ 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. The reference + 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/docs/migration-4.0.md b/docs/migration-4.0.md index e7c1e514..54dcda9f 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 355fd794..3fd61929 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. 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 + 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 d115bb43..510bde1b 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 f0812928..7478cfa5 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 509f09c8..18341bba 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,75 @@ 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_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 / 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, + 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_dr_scores.py b/tests/test_dr_scores.py index 5094b3e9..4ab91241 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 6c8510bc..09bc61b7 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 477f5bf2..15f9fcb8 100644 --- a/tests/test_event_study_consumers.py +++ b/tests/test_event_study_consumers.py @@ -2147,3 +2147,313 @@ 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.""" + + @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 + + 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_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_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_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_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). + 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 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).""" + + @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 + + 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 2dbe319b..c686c956 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 4a8514d9..7bef8a22 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 b74cbfdd..09180c4c 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 00a83584..42d58d49 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 12eab70a..c95173d2 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 23df78c6..c236a237 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 88b4634c..ced3ff9f 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 d75d3f26..b8d1a52d 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 67e4dc64..14c1cf6e 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 201d7979..2f955222 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 e84f0f47..46dab1d7 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