Skip to content

Commit f97ab76

Browse files
igerberclaude
andcommitted
Address PR #401 R6 review (1 P1 Cartesian-product test gap)
P1 (Documentation/Tests): the docstring claimed paths_of_interest composed with bootstrap, per-path placebos, joint sup-t bands, trends_linear, and trends_nonparam, but the new tests covered only the analytical trends_* paths and a triple-combo (paths_of_interest + non-binary D + bootstrap + placebo). The R6 reviewer flagged two specific gaps: 1. paths_of_interest + trends_linear=True + n_bootstrap > 0 + placebo=True: post-bootstrap path_cumulated_event_study path was untested. 2. paths_of_interest + trends_nonparam="state" + placebo=True + n_bootstrap > 0: the set_ids flow through all four per-path collectors (analytical, placebo, bootstrap, placebo-bootstrap) was untested. Plus the existing non-binary triple-combo's sup-t check was vacuous (`assert res.path_sup_t_bands is not None` is satisfied by `{}` when the strict-majority gate excludes every path). Fixes: - Add test_paths_of_interest_trends_linear_bootstrap_placebo asserting path-set restriction, finite per-path bootstrap SE on path_effects, populated post-bootstrap path_cumulated_event_study with finite (effect, SE) per (path, horizon), and populated per-path placebo for the same paths. - Add test_paths_of_interest_trends_nonparam_bootstrap_placebo asserting selector + set_ids flow through all four collectors: finite analytical/bootstrap (effect, SE) on path_effects, finite per-path placebo (effect, SE). - Tighten test_paths_of_interest_non_binary_bootstrap_placebo: bump to 2 paths and n_bootstrap=400 so the sup-t strict-majority gate fires, and assert >=1 finite crit_value AND non-NaN cband_conf_int per (path, horizon) on path_effects (replacing the vacuous "is not None" check). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8dd0e41 commit f97ab76

1 file changed

Lines changed: 141 additions & 11 deletions

File tree

tests/test_chaisemartin_dhaultfoeuille.py

Lines changed: 141 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8907,12 +8907,17 @@ def test_paths_of_interest_with_trends_nonparam(self):
89078907

89088908
@pytest.mark.slow
89098909
def test_paths_of_interest_non_binary_bootstrap_placebo(self):
8910-
"""Triple-combo: paths_of_interest + non-binary D + bootstrap + placebo."""
8910+
"""Quadruple-combo: paths_of_interest + non-binary D + bootstrap
8911+
+ placebo. Asserts (i) selector restricts path_effects to the
8912+
requested paths, (ii) bootstrap SE finite on every (path,
8913+
horizon) for the selected paths, (iii) per-path placebo
8914+
populated, (iv) at least one selected path has a finite sup-t
8915+
crit_value and the corresponding `cband_conf_int` is non-NaN."""
89118916
df = _by_path_data_with_non_binary_treatment()
89128917
est = ChaisemartinDHaultfoeuille(
89138918
drop_larger_lower=False,
8914-
paths_of_interest=[(0, 2, 2, 2)],
8915-
n_bootstrap=200,
8919+
paths_of_interest=[(0, 2, 2, 2), (0, 1, 1, 1)],
8920+
n_bootstrap=400,
89168921
placebo=True,
89178922
twfe_diagnostic=False,
89188923
seed=42,
@@ -8923,19 +8928,144 @@ def test_paths_of_interest_non_binary_bootstrap_placebo(self):
89238928
df, outcome="outcome", group="group", time="period",
89248929
treatment="treatment", L_max=3,
89258930
)
8926-
assert set(res.path_effects.keys()) == {(0, 2, 2, 2)}
8927-
# All four surfaces populated:
8931+
assert set(res.path_effects.keys()) == {(0, 2, 2, 2), (0, 1, 1, 1)}
89288932
# 1. analytical / bootstrap on path_effects (SE finite)
8929-
for l_h, vals in res.path_effects[(0, 2, 2, 2)]["horizons"].items():
8930-
assert np.isfinite(vals["effect"])
8931-
assert np.isfinite(vals["se"])
8933+
for path in res.path_effects:
8934+
for l_h, vals in res.path_effects[path]["horizons"].items():
8935+
assert np.isfinite(vals["effect"]), f"path={path} l={l_h}"
8936+
assert np.isfinite(vals["se"]), f"path={path} l={l_h}"
89328937
# 2. per-path placebo
89338938
assert res.path_placebo_event_study is not None
89348939
assert (0, 2, 2, 2) in res.path_placebo_event_study
8935-
# 3. per-path sup-t bands (only one path so the strict-majority
8936-
# gate may or may not fire depending on n_bootstrap; check
8937-
# that the structure exists)
8940+
assert (0, 1, 1, 1) in res.path_placebo_event_study
8941+
# 3. per-path sup-t bands: at least one selected path passes the
8942+
# strict-majority gate with a finite crit_value AND the
8943+
# corresponding cband_conf_int entries on path_effects are
8944+
# non-NaN tuples (vacuous "is not None" check rejected by R6).
89388945
assert res.path_sup_t_bands is not None
8946+
finite_crit_paths = [
8947+
p
8948+
for p, entry in res.path_sup_t_bands.items()
8949+
if np.isfinite(entry.get("crit_value", np.nan))
8950+
]
8951+
assert len(finite_crit_paths) >= 1, (
8952+
f"Expected >=1 selected path with finite sup-t crit; "
8953+
f"got path_sup_t_bands={res.path_sup_t_bands}"
8954+
)
8955+
# cband_conf_int populated for positive horizons of finite-crit paths.
8956+
for path in finite_crit_paths:
8957+
for l_h in range(1, 4):
8958+
cband = res.path_effects[path]["horizons"][l_h].get(
8959+
"cband_conf_int"
8960+
)
8961+
assert cband is not None, f"path={path} l={l_h}: cband missing"
8962+
lo, hi = cband
8963+
assert np.isfinite(lo) and np.isfinite(hi), (
8964+
f"path={path} l={l_h}: cband endpoints not finite "
8965+
f"(lo={lo}, hi={hi})"
8966+
)
8967+
8968+
@pytest.mark.slow
8969+
def test_paths_of_interest_trends_linear_bootstrap_placebo(self):
8970+
"""`paths_of_interest + trends_linear=True + n_bootstrap > 0 +
8971+
placebo=True`: assert (i) selector restricts the path set, (ii)
8972+
per-path bootstrap SE on `path_effects` finite, (iii)
8973+
post-bootstrap `path_cumulated_event_study` populated for the
8974+
same paths (mirrors global `linear_trends_effects` cumulation),
8975+
(iv) per-path placebo populated. Regression for the R6 P1
8976+
Cartesian-product gap."""
8977+
df = _by_path_data_with_trends_linear()
8978+
user_paths = [(0, 1, 1, 1), (0, 1, 1, 0)]
8979+
est = ChaisemartinDHaultfoeuille(
8980+
drop_larger_lower=False,
8981+
paths_of_interest=user_paths,
8982+
n_bootstrap=200,
8983+
placebo=True,
8984+
twfe_diagnostic=False,
8985+
seed=42,
8986+
)
8987+
with warnings.catch_warnings():
8988+
warnings.simplefilter("ignore", UserWarning)
8989+
res = est.fit(
8990+
df, outcome="outcome", group="group", time="period",
8991+
treatment="treatment", L_max=3, trends_linear=True,
8992+
)
8993+
# (i) selector restricts the path set
8994+
assert set(res.path_effects.keys()) == set(user_paths)
8995+
# (ii) per-path bootstrap SE finite on event study
8996+
for path in res.path_effects:
8997+
for l_h, vals in res.path_effects[path]["horizons"].items():
8998+
assert np.isfinite(vals["se"]), (
8999+
f"path={path} l={l_h}: bootstrap SE not finite"
9000+
)
9001+
# (iii) post-bootstrap path_cumulated_event_study populated for
9002+
# the same paths AND derived from the post-bootstrap per-horizon
9003+
# SEs (cumulated SE = sum of post-bootstrap component SEs).
9004+
assert res.path_cumulated_event_study is not None
9005+
assert set(res.path_cumulated_event_study.keys()) == set(user_paths)
9006+
for path in user_paths:
9007+
assert len(res.path_cumulated_event_study[path]) > 0
9008+
for l_h, vals in res.path_cumulated_event_study[path].items():
9009+
assert np.isfinite(vals["effect"]), (
9010+
f"path={path} l={l_h}: cumulated effect not finite"
9011+
)
9012+
assert np.isfinite(vals["se"]), (
9013+
f"path={path} l={l_h}: cumulated SE not finite"
9014+
)
9015+
# (iv) per-path placebo populated
9016+
assert res.path_placebo_event_study is not None
9017+
assert set(res.path_placebo_event_study.keys()) == set(user_paths)
9018+
9019+
@pytest.mark.slow
9020+
def test_paths_of_interest_trends_nonparam_bootstrap_placebo(self):
9021+
"""`paths_of_interest + trends_nonparam="state" + placebo=True +
9022+
n_bootstrap > 0`: assert the selector + set_ids flow through
9023+
the four per-path collectors (`_compute_path_effects`,
9024+
`_compute_path_placebos`, `_collect_path_bootstrap_inputs`,
9025+
`_collect_path_placebo_bootstrap_inputs`) and the resulting
9026+
public surfaces are populated. Regression for the R6 P1
9027+
Cartesian-product gap."""
9028+
df = _by_path_data_with_trends_nonparam()
9029+
user_paths = [(0, 1, 1, 1), (0, 1, 1, 0)]
9030+
est = ChaisemartinDHaultfoeuille(
9031+
drop_larger_lower=False,
9032+
paths_of_interest=user_paths,
9033+
n_bootstrap=200,
9034+
placebo=True,
9035+
twfe_diagnostic=False,
9036+
seed=42,
9037+
)
9038+
with warnings.catch_warnings():
9039+
warnings.simplefilter("ignore", UserWarning)
9040+
res = est.fit(
9041+
df, outcome="outcome", group="group", time="period",
9042+
treatment="treatment", L_max=3,
9043+
trends_nonparam="state",
9044+
)
9045+
# Selector restriction
9046+
assert set(res.path_effects.keys()) == set(user_paths)
9047+
# Per-path bootstrap SE finite on event study (bootstrap
9048+
# collector + set_ids reached path_effects)
9049+
for path in res.path_effects:
9050+
for l_h, vals in res.path_effects[path]["horizons"].items():
9051+
assert np.isfinite(vals["effect"]), f"path={path} l={l_h}"
9052+
assert np.isfinite(vals["se"]), f"path={path} l={l_h}"
9053+
# Per-path placebo populated and bootstrap SE finite
9054+
# (placebo collector + set_ids reached path_placebo_event_study)
9055+
assert res.path_placebo_event_study is not None
9056+
assert set(res.path_placebo_event_study.keys()) == set(user_paths)
9057+
any_finite_placebo_se = False
9058+
for path in user_paths:
9059+
for lag, vals in res.path_placebo_event_study[path].items():
9060+
if np.isfinite(vals["effect"]) and np.isfinite(vals["se"]):
9061+
any_finite_placebo_se = True
9062+
break
9063+
if any_finite_placebo_se:
9064+
break
9065+
assert any_finite_placebo_se, (
9066+
"No finite (effect, SE) pair on per-path placebo surface "
9067+
"under paths_of_interest + trends_nonparam + bootstrap"
9068+
)
89399069

89409070
# ---- single-surface inheritance (slow) ----
89419071

0 commit comments

Comments
 (0)