Skip to content

Commit 450c592

Browse files
igerberclaude
andcommitted
Address PR #347 R3: sync docs to implemented aggregation tags + real-fit no-scalar test
R3 approved (✅) with two non-blocking follow-ups; this commit addresses both. P2 (docs): ``REPORTING.md`` and ``business_report.rst`` still listed the obsolete dCDH aggregation tags (``"DID_l"``, ``"l"``, ``"l_x"``, ``"l_fd"``, ``"l_x_fd"``) and documented ``headline_attribute`` as always a string, even though R2 replaced those with ``"DID_1"`` / ``"DID_1_x"`` / ``"DID_1_fd"`` / ``"DID_1_x_fd"`` / ``"delta"`` / ``"delta_x"`` / ``"no_scalar_headline"`` and introduced the ``headline_attribute=None`` no-scalar case. Consumers wiring dispatch logic off the docs would have pointed at tags the helper no longer emits. Rewrote the ``aggregation`` enum in REPORTING.md as a full per-estimator dispatch list, and updated the ``headline_attribute`` description to name the ``None`` case explicitly. ``business_report.rst`` summary replaced ``DID_l`` with ``DID_1`` / cost-benefit delta and added a pointer to the no-scalar case. P3 (tests): added ``test_dcdh_trends_linear_with_l_max_geq_2_fit_real`` — a real-fit regression that exercises the ``ChaisemartinDHaultfoeuille(..., L_max=2, trends_linear=True)`` path end-to-end. Asserts (a) ``fit.overall_att`` is NaN by design (matching ``chaisemartin_dhaultfoeuille.py:2828-2834``), (b) ``linear_trends_effects`` is populated, (c) the target-parameter block emits ``aggregation="no_scalar_headline"`` and ``headline_attribute is None``, (d) the definition references ``linear_trends_effects``. Previously this branch was only stub-tested; now the reporting-layer integration is pinned by a live dCDH fit. 333 BR/DR tests pass. Black and ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9f6b4d1 commit 450c592

3 files changed

Lines changed: 79 additions & 11 deletions

File tree

docs/api/business_report.rst

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,16 @@ stability) are documented in :doc:`../methodology/REPORTING`.
5151

5252
The schema carries a top-level ``target_parameter`` block
5353
(experimental) naming what the headline scalar represents per
54-
estimator — simple ATT, event-study average, DID_M, DID_l,
55-
dose-response aggregate, factor-model residual, etc. See the
56-
"Target parameter" section of :doc:`../methodology/REPORTING` for
57-
the per-estimator dispatch and schema shape.
54+
estimator — simple ATT, event-study average, DID_M, DID_1,
55+
cost-benefit delta, dose-response aggregate, factor-model residual,
56+
etc. For the dCDH dynamic branch with ``trends_linear=True`` and
57+
``L_max>=2``, the scalar is intentionally NaN and
58+
``aggregation`` is ``"no_scalar_headline"`` with
59+
``headline_attribute`` set to ``None`` — agents should dispatch on
60+
this case and consult ``linear_trends_effects`` instead of
61+
``overall_att``. See the "Target parameter" section of
62+
:doc:`../methodology/REPORTING` for the full per-estimator dispatch
63+
table and schema shape.
5864

5965
Example
6066
-------

docs/methodology/REPORTING.md

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,35 @@ Field semantics:
8888
summary paragraph so stakeholder prose stays within the 6-10-
8989
sentence target.
9090
- `aggregation` — machine-readable tag dispatching agents can
91-
branch on: `"simple"`, `"event_study"`, `"group"`, `"2x2"`,
92-
`"twfe"`, `"iw"`, `"stacked"`, `"ddd"`, `"staggered_ddd"`,
93-
`"synthetic"`, `"factor_model"`, `"M"`, `"l"`, `"l_x"`,
94-
`"l_fd"`, `"l_x_fd"`, `"dose_overall"`,
95-
`"pt_all_combined"`, `"pt_post_single_baseline"`, `"unknown"`.
91+
branch on. Complete enumeration per estimator:
92+
- `"2x2"` (DiDResults / TwoWayFixedEffects both route here)
93+
- `"event_study"` (MultiPeriodDiDResults)
94+
- `"simple"` (CallawaySantAnna / Imputation / TwoStage / Wooldridge)
95+
- `"iw"` (SunAbraham)
96+
- `"stacked"` (StackedDiD)
97+
- `"pt_all_combined"` / `"pt_post_single_baseline"` (EfficientDiD
98+
branched on `pt_assumption`)
99+
- `"dose_overall"` (ContinuousDiD)
100+
- `"ddd"` / `"staggered_ddd"` (TripleDifference / StaggeredTripleDiff)
101+
- dCDH dynamic branches follow the exact `overall_att`
102+
contract: `"M"` / `"M_x"` / `"M_fd"` / `"M_x_fd"` for
103+
`L_max=None`; `"DID_1"` / `"DID_1_x"` / `"DID_1_fd"` /
104+
`"DID_1_x_fd"` for `L_max=1`; `"delta"` / `"delta_x"` for
105+
`L_max>=2` without trend suppression; and
106+
`"no_scalar_headline"` when `trends_linear=True` AND
107+
`L_max>=2` (the scalar is intentionally NaN).
108+
- `"synthetic"` (SyntheticDiD) / `"factor_model"` (TROP) /
109+
`"twfe"` (BaconDecomposition read-out) / `"unknown"` (default
110+
fallback).
96111
- `headline_attribute` — the raw result attribute the scalar
97112
comes from (`"overall_att"` / `"att"` / `"avg_att"` /
98-
`"twfe_estimate"`). Different result classes use different
99-
attribute names; agents that want to re-read the raw value
113+
`"twfe_estimate"`), OR `None` when `aggregation ==
114+
"no_scalar_headline"` (the dCDH `trends_linear=True,
115+
L_max>=2` branch where `overall_att` is intentionally NaN by
116+
design). Agents dispatching on this field must handle `None` as
117+
"no scalar aggregate — consult `linear_trends_effects`
118+
instead". Different result classes use different attribute
119+
names; agents that want to re-read the raw value
100120
can dispatch on this.
101121
- `reference` — one-line citation pointer to the canonical paper
102122
and the REGISTRY.md section.

tests/test_target_parameter.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,3 +588,45 @@ def test_dcdh_delta_fit_real(self):
588588
assert "delta" in tp["name"]
589589
assert tp["headline_attribute"] == "overall_att"
590590
assert hasattr(fit, "overall_att")
591+
592+
def test_dcdh_trends_linear_with_l_max_geq_2_fit_real(self):
593+
"""Real ``trends_linear=True`` + ``L_max>=2`` fit: the library
594+
intentionally sets ``overall_att=NaN`` and populates the
595+
per-horizon effects on ``linear_trends_effects`` instead
596+
(``chaisemartin_dhaultfoeuille.py:2828-2834``). The target-
597+
parameter block must reflect that via
598+
``aggregation="no_scalar_headline"`` and
599+
``headline_attribute is None``. PR #347 R3 P3 regression on
600+
a live fit.
601+
"""
602+
import math
603+
import warnings
604+
605+
from diff_diff import ChaisemartinDHaultfoeuille
606+
607+
warnings.filterwarnings("ignore")
608+
df = self._dcdh_reversible_panel(seed=15)
609+
fit = ChaisemartinDHaultfoeuille().fit(
610+
df,
611+
outcome="outcome",
612+
group="unit",
613+
time="period",
614+
treatment="treated",
615+
L_max=2,
616+
trends_linear=True,
617+
)
618+
# Real fit must intentionally produce NaN overall_att.
619+
assert math.isnan(fit.overall_att), (
620+
"trends_linear + L_max>=2 must suppress overall_att (NaN by "
621+
"design). If this test fails, the library contract changed — "
622+
"update the ``no_scalar_headline`` branch of "
623+
"describe_target_parameter accordingly."
624+
)
625+
# linear_trends_effects must be populated (the per-horizon
626+
# cumulated level effects that replace the scalar aggregate).
627+
assert fit.linear_trends_effects is not None
628+
# Target-parameter block must route through the no-scalar branch.
629+
tp = describe_target_parameter(fit)
630+
assert tp["aggregation"] == "no_scalar_headline"
631+
assert tp["headline_attribute"] is None
632+
assert "linear_trends_effects" in tp["definition"]

0 commit comments

Comments
 (0)