Skip to content

Commit af998df

Browse files
igerberclaude
andcommitted
Address PR #376 R4 (1 P1 + 1 P3)
R4 P1: HeterogeneousAdoptionDiD.fit() inadvertently made `survey`, `weights`, and `cband` keyword-only when adding the new `survey_design=` kwarg, by inserting `*,` before all four. This broke pre-PR positional callers, contradicting the "additive, non-breaking" CHANGELOG claim. Reorder so `survey`, `weights`, `cband` keep their pre-PR positional-or-keyword status; only `survey_design=` is the new keyword-only addition (placed after the `*,` separator at the end). R4 P3: added test_legacy_positional_call_back_compat in TestHADFitDeprecation that exercises the full pre-PR positional call shape: `fit(df, "y", "d", "time", "unit", None, "overall", sd, None, True)` — locks the back-compat contract. The 6 array-in pretest helpers + workflow + qug_test were unaffected by this issue: their pre-PR signatures already had `*,` before survey/ weights/etc, so those kwargs were already keyword-only. 551 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 03c5c9d commit af998df

2 files changed

Lines changed: 32 additions & 2 deletions

File tree

diff_diff/had.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,11 +2789,15 @@ def fit(
27892789
unit_col: str,
27902790
first_treat_col: Optional[str] = None,
27912791
aggregate: str = "overall",
2792-
*,
2793-
survey_design: Any = None,
2792+
# PR #376 R4 P1: preserve pre-PR positional-or-keyword status of
2793+
# `survey`, `weights`, `cband` for back-compat with positional
2794+
# callers. `survey_design=` is the only new addition and is
2795+
# keyword-only.
27942796
survey: Any = None,
27952797
weights: Optional[np.ndarray] = None,
27962798
cband: bool = True,
2799+
*,
2800+
survey_design: Any = None,
27972801
) -> HeterogeneousAdoptionDiDResults:
27982802
"""Fit the HAD estimator.
27992803

tests/test_had_dual_knob_deprecation.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,32 @@ def test_three_way_mutex_design_plus_weights(self, two_period_panel):
692692
weights=np.ones(n),
693693
)
694694

695+
def test_legacy_positional_call_back_compat(self, two_period_panel):
696+
"""PR #376 R4 P1: pre-PR positional call shape for `survey`,
697+
`weights`, `cband` MUST still work (the consolidation is additive,
698+
not breaking). Tests the full positional sequence:
699+
`fit(data, outcome, dose, time, unit, first_treat, aggregate,
700+
survey, weights, cband)`."""
701+
df = two_period_panel
702+
est = HeterogeneousAdoptionDiD(design="continuous_at_zero")
703+
# Pre-PR positional order: ..., first_treat_col, aggregate, survey,
704+
# weights, cband. None of these should be flagged as keyword-only.
705+
with warnings.catch_warnings():
706+
warnings.simplefilter("ignore", DeprecationWarning)
707+
r = est.fit(
708+
df,
709+
"y",
710+
"d",
711+
"time",
712+
"unit",
713+
None, # first_treat_col
714+
"overall", # aggregate
715+
SurveyDesign(weights="w"), # survey (positional)
716+
None, # weights (positional)
717+
True, # cband (positional)
718+
)
719+
assert np.isfinite(r.att)
720+
695721

696722
class TestDidHadPretestWorkflowDeprecation:
697723
def test_survey_design_kwarg_smoke(self, two_period_panel):

0 commit comments

Comments
 (0)