Skip to content

Commit 0040bad

Browse files
igerberclaude
andcommitted
Address PR #353 CI review round 3 (1 P1 + 1 P3)
P1 - row-level non-negative-dose guard in `_aggregate_for_joint_test`: On a 2-period direct call to `joint_pretrends_test` or `joint_homogeneity_test`, the n_periods < 3 path skips `_validate_had_panel_event_study` (which requires >= 3 periods) and falls through to `_aggregate_for_joint_test`. That helper collapsed unit dose via `groupby(unit_col)[dose_col].max()`, which silently recodes a negative post dose to 0 (`max(0, -d) = 0` for positive pre-period d), allowing finite joint-Stute output on data that violates the HAD support restriction `D_{g,t} >= 0` (paper Section 2). Fix: add a row-level `dose_col >= 0` check in `_aggregate_for_joint_test` BEFORE the groupby/max collapse. Centralizes the guard so both data-in wrappers inherit it on the n_periods < 3 fallback path. The multi-period path already enforces the same invariant via `_validate_had_panel_event_study`, so the contract is consistent across both wrapper dispatch modes. P3 - regression test: new `TestJointHomogeneityTest::test_two_period_negative_post_dose_raises` constructs a 2-period panel with a single unit carrying a negative post dose and asserts the wrapper raises `ValueError` with the "negative dose value" substring rather than producing a finite statistic via the groupby-max collapse. 124 tests pass (123 + 1 new R3 regression); black/ruff/mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 84835de commit 0040bad

2 files changed

Lines changed: 59 additions & 0 deletions

File tree

diff_diff/had_pretests.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,26 @@ def _aggregate_for_joint_test(
17371737
f"silently drop rows; drop or impute before calling."
17381738
)
17391739

1740+
# Row-level non-negative-dose guard (paper Section 2 HAD support
1741+
# restriction `D_{g,t} >= 0`). Must run BEFORE the groupby/max()
1742+
# collapse below, otherwise a negative post dose would silently
1743+
# become 0 in the per-unit dose vector (since `max(0, -d) = 0` for
1744+
# positive d), letting the wrappers run on invalid data and
1745+
# potentially return finite results. This is the direct-wrapper
1746+
# equivalent of the row-level check inside
1747+
# `_validate_had_panel_event_study`, centralized so both
1748+
# `joint_pretrends_test` and `joint_homogeneity_test` inherit it on
1749+
# the `n_periods < 3` fallback path that skips the validator.
1750+
negative_dose_mask = subset[dose_col] < 0
1751+
if bool(negative_dose_mask.any()):
1752+
n_neg = int(negative_dose_mask.sum())
1753+
raise ValueError(
1754+
f"{n_neg} negative dose value(s) found in column "
1755+
f"{dose_col!r} across periods {needed_periods}. HAD support "
1756+
f"restriction (paper Section 2) requires D_{{g,t}} >= 0 "
1757+
f"for every (unit, period)."
1758+
)
1759+
17401760
counts = subset.groupby(unit_col).size()
17411761
n_needed = len(needed_periods)
17421762
if (counts != n_needed).any():

tests/test_had_pretests.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,6 +2035,45 @@ def test_all_zero_dose_post_period_raises(self):
20352035
seed=0,
20362036
)
20372037

2038+
def test_two_period_negative_post_dose_raises(self):
2039+
"""R2 P1 regression: direct wrapper call on a 2-period panel
2040+
with a negative post dose must raise rather than silently
2041+
collapse to zero via ``groupby.max()`` and produce a finite
2042+
result. The 2-period path skips the event-study validator
2043+
(``n_periods < 3``) so the row-level non-negative guard must
2044+
live in ``_aggregate_for_joint_test`` itself."""
2045+
G = 20
2046+
rng = np.random.default_rng(601)
2047+
doses = rng.uniform(0.1, 1.0, size=G)
2048+
# Flip one unit's post dose to a negative value.
2049+
doses[0] = -0.3
2050+
rows = []
2051+
for g in range(G):
2052+
# pre-period
2053+
rows.append({"unit": g, "period": 0, "y": rng.normal(0, 0.1), "d": 0.0})
2054+
# post-period (with negative dose injected for unit 0)
2055+
rows.append(
2056+
{
2057+
"unit": g,
2058+
"period": 1,
2059+
"y": rng.normal(0, 0.1) + 0.3 * doses[g],
2060+
"d": float(doses[g]),
2061+
}
2062+
)
2063+
df = pd.DataFrame(rows)
2064+
with pytest.raises(ValueError, match="negative dose value"):
2065+
joint_homogeneity_test(
2066+
df,
2067+
"y",
2068+
"d",
2069+
"period",
2070+
"unit",
2071+
post_periods=[1],
2072+
base_period=0,
2073+
n_bootstrap=199,
2074+
seed=0,
2075+
)
2076+
20382077

20392078
class TestMultiPeriodWorkflow:
20402079
"""Tests for :func:`did_had_pretest_workflow` event-study dispatch."""

0 commit comments

Comments
 (0)