Skip to content

Commit d992779

Browse files
igerberclaude
andcommitted
Address PR #192 review (Round 5): balance_e empty warning, inline comments for repeat false positives
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1a9ff9b commit d992779

File tree

3 files changed

+45
-2
lines changed

3 files changed

+45
-2
lines changed

diff_diff/efficient_did.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,11 @@ def fit(
319319
else:
320320
effective_p1_col = period_1_col
321321

322+
# Estimate all (g, t) cells including pre-treatment. Under PT-Post,
323+
# pre-treatment cells serve as placebo/pre-trend diagnostics, matching
324+
# the CallawaySantAnna implementation. Users filter to t >= g for
325+
# post-treatment effects; pre-treatment cells are clearly labeled by
326+
# their (g, t) coordinates in the results object.
322327
for t in time_periods:
323328
# Skip period_1 — it's the universal reference baseline,
324329
# not a target period
@@ -713,6 +718,14 @@ def _aggregate_event_study(
713718
balanced[e].append(((g, t), data["effect"], cohort_fractions.get(g, 0.0)))
714719
effects_by_e = balanced
715720

721+
if balance_e is not None and not effects_by_e:
722+
warnings.warn(
723+
f"balance_e={balance_e}: no cohort has a finite effect at the "
724+
"anchor horizon. Event study will be empty.",
725+
UserWarning,
726+
stacklevel=2,
727+
)
728+
716729
result: Dict[int, Dict[str, Any]] = {}
717730
for e, elist in sorted(effects_by_e.items()):
718731
gt_pairs = [x[0] for x in elist]

diff_diff/efficient_did_bootstrap.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,12 @@ def _run_multiplier_bootstrap(
116116
)
117117
post_indices = np.where(post_mask)[0]
118118

119-
# Overall ATT aggregation weights (cohort-size)
119+
# Overall ATT: fixed-weight re-aggregation of perturbed cell ATTs.
120+
# This matches CallawaySantAnna._run_multiplier_bootstrap
121+
# (staggered_bootstrap.py:281). The analytical path includes a WIF
122+
# correction; bootstrap captures sampling variability through per-cell
123+
# EIF perturbation without re-estimating weights — this is standard
124+
# in both this library's CS implementation and the R did package.
120125
skip_overall = len(post_indices) == 0
121126
if skip_overall:
122127
bootstrap_overall = np.full(self.n_bootstrap, np.nan)
@@ -129,7 +134,8 @@ def _run_multiplier_bootstrap(
129134
with np.errstate(divide="ignore", invalid="ignore", over="ignore"):
130135
bootstrap_overall = bootstrap_atts[:, post_indices] @ agg_w
131136

132-
# Event study aggregation
137+
# Event study: fixed-weight re-aggregation (same pattern as overall).
138+
# See note above re: WIF — analytical WIF is not needed in bootstrap.
133139
bootstrap_event_study = None
134140
event_study_info = None
135141
if aggregate in ("event_study", "all"):
@@ -261,6 +267,14 @@ def _prepare_es_agg_boot(
261267
balanced[e].append((j, original_atts[j], cohort_fractions.get(g, 0.0)))
262268
effects_by_e = balanced
263269

270+
if balance_e is not None and not effects_by_e:
271+
warnings.warn(
272+
f"balance_e={balance_e}: no cohort has a finite effect at the "
273+
"anchor horizon. Event study will be empty.",
274+
UserWarning,
275+
stacklevel=2,
276+
)
277+
264278
result = {}
265279
for e, elist in effects_by_e.items():
266280
indices = np.array([x[0] for x in elist])

tests/test_efficient_did.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,22 @@ def test_balance_e_nan_anchor_filters_group(self):
719719
f"Group 5 (NaN at anchor) should be excluded at e={e}, " f"got groups {groups_in_e}"
720720
)
721721

722+
def test_balance_e_empty_warns(self):
723+
"""When no cohort survives the anchor horizon, warn the user."""
724+
edid = EfficientDiD()
725+
edid.anticipation = 0
726+
727+
# All effects are NaN at e=0
728+
gt_pairs = [(3.0, 3), (3.0, 4), (5.0, 5), (5.0, 6)]
729+
original_atts = np.array([np.nan, 1.5, np.nan, 0.8])
730+
cohort_fractions = {3.0: 0.4, 5.0: 0.3}
731+
732+
with pytest.warns(UserWarning, match="no cohort has a finite effect"):
733+
result = edid._prepare_es_agg_boot(
734+
gt_pairs, original_atts, cohort_fractions, balance_e=0
735+
)
736+
assert result == {}
737+
722738

723739
# =============================================================================
724740
# Tier 3: Bootstrap

0 commit comments

Comments
 (0)