Skip to content

refactor(had): remove deprecated survey=/weights= kwargs from HeterogeneousAdoptionDiD.fit (3.7.0)#633

Merged
igerber merged 4 commits into
mainfrom
refactor/had-drop-survey-weights-kwargs
Jul 7, 2026
Merged

refactor(had): remove deprecated survey=/weights= kwargs from HeterogeneousAdoptionDiD.fit (3.7.0)#633
igerber merged 4 commits into
mainfrom
refactor/had-drop-survey-weights-kwargs

Conversation

@igerber

@igerber igerber commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Remove the deprecated survey= and weights= kwargs from HeterogeneousAdoptionDiD.fit(); survey_design= is now the sole weighting entry (design-based Binder-1983 Taylor-linearization variance), matching CallawaySantAnna / EfficientDiD / ImputationDiD / TwoStageDiD.
  • cband is now keyword-only (it previously followed the removed positional survey/weights slots).
  • The pweight / CCT-2014 dispatch branches are provably dead once weights= is gone (weights_unit_full is not None iff resolved_survey_unit_full is not None), so they are deleted along with the metadata-clearing and pweight variance_formula labels. variance_formula is now one of None / "survey_binder_tsl" / "survey_binder_tsl_2sls". The survey_design= and unweighted paths are byte-identical; ATT is unchanged on every surface (net -717 lines: dead branches + obsolete tests removed).
  • Scope: HeterogeneousAdoptionDiD.fit() only. The 7 HAD pretest helpers keep the deprecated aliases pending a follow-up removal (each has its own weight back-end; tracked in TODO.md).

Migration

  • survey=SurveyDesign(...) -> survey_design=SurveyDesign(...) (pure rename; byte-identical output).
  • fit(weights=<array>) -> add the weights as a column + survey_design=SurveyDesign(weights='col'). This is a variance-family change: pweight-robust / Normal -> Binder-TSL / t(df_survey) (SE moves ~0.1%+; ATT unchanged).
  • fit(weights=w, cluster=c) -> survey_design=SurveyDesign(weights='w', psu='c') (Binder-TSL clustered through the PSU).

Methodology references (required if estimator / math changes)

  • Method name(s): HeterogeneousAdoptionDiD — de Chaisemartin, Ciccia, D'Haultfœuille & Knau (2026), arXiv:2405.04465v6; survey variance via Binder (1983) Taylor-series linearization through the shared compute_survey_if_variance.
  • Paper / source link(s): arXiv:2405.04465v6; Binder (1983), International Statistical Review 51(3).
  • Any intentional deviations from the source (and why): This is a pre-scheduled public-kwarg removal in a minor (3.7.0) bump, documented since the DeprecationWarning shipped ("will be removed in the next minor release"). The fit(weights=<array>) -> survey_design= migration is a documented variance-family change (pweight/Normal -> Binder-TSL/t(df)). See CHANGELOG ### Removed and REGISTRY "Note (HAD survey-design API consolidation)".

Validation

  • Tests added/updated: reworked test_had_dual_knob_deprecation.py (removed kwargs -> TypeError; cband keyword-only; obsolete alias-parity tests dropped); migrated the weighted-behavior tests in test_had.py, test_had_mc.py, test_methodology_had.py, test_had_pretests.py from weights=/survey= to survey_design= (ATT bit-parity kept; SE relaxed to finite/positive only where the survey variance family legitimately differs); updated test_guides.py / test_practitioner.py enumeration guards (variance_formula labels 4 -> 2). Full HAD suite green; mypy at baseline (183); black/ruff clean.
  • Backtest / simulation / notebook evidence (if applicable): N/A — no tutorial changes (verified no notebook code cell calls the removed kwargs).

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi

…eneousAdoptionDiD.fit (3.7.0)

Remove the deprecated `survey=` and `weights=` kwargs from
`HeterogeneousAdoptionDiD.fit()`; `survey_design=` is now the sole weighting
entry (design-based Binder-1983 Taylor-linearization variance), matching
CallawaySantAnna / EfficientDiD / ImputationDiD / TwoStageDiD. `cband` is now
keyword-only (it followed the removed positional slots).

The pweight / CCT-2014 dispatch branches are provably dead once `weights=` is
gone (weights_unit_full is not None iff resolved_survey_unit_full is not None),
so they are deleted along with the metadata-clearing and pweight variance_formula
labels. `variance_formula` is now one of None / "survey_binder_tsl" /
"survey_binder_tsl_2sls". The survey_design= and unweighted paths are unchanged
(byte-identical); ATT is unchanged on every surface.

Migration: `survey=SurveyDesign(...)` -> `survey_design=SurveyDesign(...)` (pure
rename); `fit(weights=<array>)` -> weight column + `survey_design=SurveyDesign(
weights='col')` (SE moves from pweight/Normal to Binder-TSL/t(df); ~0.1%+);
`fit(weights=w, cluster=c)` -> `survey_design=SurveyDesign(weights='w', psu='c')`.

Docs (CHANGELOG ### Removed, REGISTRY notes, had.rst, llms-full, troubleshooting)
and tests are updated to the survey_design=-only contract; the clustering-migration
guidance is `survey_design=SurveyDesign(weights=, psu=)` (a HAD survey design
without a weights column is rejected). Removed-kwarg tests now assert TypeError.

Scope: HeterogeneousAdoptionDiD.fit() only. The 7 HAD pretest helpers keep the
deprecated aliases pending a follow-up removal (each has its own weight back-end;
tracked in TODO.md).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • Affected method: HeterogeneousAdoptionDiD.fit() across continuous and mass-point, overall and event-study paths.
  • The survey= / weights= removal and Binder-TSL-only weighting contract are documented in docs/methodology/REGISTRY.md:L3186-L3187.
  • I did not find a surviving inline inference or partial NaN-guard issue; modified paths use safe_inference() at diff_diff/had.py:L3771-L3780 and diff_diff/had.py:L4589-L4597.
  • Remaining HAD pretest helper alias removal is tracked in TODO.md:L137, so it is not a blocker.
  • Could not run tests locally: this environment lacks pytest and numpy.

Methodology

  • Severity: P3 informational
    Impact: The removal of fit(survey=...) / fit(weights=...), removal of pweight variance labels, and consolidation onto survey_design= / Binder-TSL are documented deviations, not defects. The runtime also rejects mass-point vcov_type="classical" under survey and rejects cluster= + survey_design= rather than silently mixing variance families (diff_diff/had.py:L3678-L3690, diff_diff/had.py:L3714-L3725, diff_diff/had.py:L4407-L4422).
    Concrete fix: None required.

Code Quality

No findings.

Performance

No findings. Removing dead pweight branches should reduce branching and avoids extra synthetic survey metadata construction.

Maintainability

No blocking findings. The remaining pretest-helper cleanup is explicitly tracked in TODO.md:L137.

Tech Debt

  • Severity: P3 informational
    Impact: The seven HAD pretest helper aliases remain, but the PR tracks that deferred work in TODO.md:L137; per review policy this is non-blocking.
    Concrete fix: None required for this PR.

Security

No findings.

Documentation/Tests

  • Severity: P3
    Impact: A few result-field descriptions still say the mass-point "survey_binder_tsl_2sls" label is applied across “classical / HC1 / CR1” vcov families, but the runtime rejects classical survey and rejects bare cluster= + survey_design=. This is documentation drift only, not a statistical-output bug. See diff_diff/had.py:L366-L372, diff_diff/had.py:L503-L509, diff_diff/guides/llms-full.txt:L1558-L1558, and diff_diff/guides/llms-full.txt:L1591-L1591.
    Concrete fix: Reword those descriptions to say the label is emitted on supported mass-point survey paths, with vcov_type="hc1" / robust=True; PSU clustering is represented through SurveyDesign(psu=...), not vcov_type="cr1".

…the mass-point survey path)

The mass-point `survey_design=` path emits `variance_formula="survey_binder_tsl_2sls"`
only under `vcov_type="hc1"` / `robust=True`: it rejects `vcov_type="classical"`
(had.py:3714) and rejects `cluster=` + `survey_design=` (had.py:3678), so PSU
clustering is expressed via `SurveyDesign(psu=...)`, not `vcov_type="cr1"`. The
result-field / to_dict docstrings and the llms-full variance_formula rows wrongly
carried the old pweight_2sls "applied uniformly across classical / HC1 / CR1"
framing onto the survey_binder_tsl_2sls label; reword to the hc1-only contract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: b208d53f802ab06a9e8f755ed4ba6b6f23b3e300


Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • Affected method: HeterogeneousAdoptionDiD.fit() static and event-study paths, across continuous and mass-point survey-weighted inference.
  • The survey= / weights= removal, cband keyword-only change, and Binder-TSL-only weighting contract are documented in docs/methodology/REGISTRY.md:L3186-L3187.
  • I found no surviving inline inference anti-patterns; modified inference paths use safe_inference() at diff_diff/had.py:L3779-L3782 and diff_diff/had.py:L4591-L4593.
  • Mass-point survey/classical and cluster= + survey_design= restrictions remain documented and fail-closed, matching the registry at docs/methodology/REGISTRY.md:L3213-L3215.
  • Remaining pretest-helper alias removal is tracked in TODO.md:L137, so it is not a blocker.

Methodology

  • Severity: P3 informational
    Impact: The public API and variance-family change is a documented deviation/consolidation, not a methodology defect. fit() now exposes only survey_design= with cband keyword-only (diff_diff/had.py:L2972-L2985), and the registry explicitly records the removal plus the remaining valid variance_formula labels.
    Concrete fix: None required.

Code Quality

No findings.

Performance

No findings. Removing the dead pweight branches reduces conditional surface area.

Maintainability

No blocking findings. The remaining seven HAD pretest-helper aliases are explicitly tracked as deferred work in TODO.md:L137.

Tech Debt

  • Severity: P3 informational
    Impact: HAD pretest helper cleanup remains deferred and tracked, with scope separated from HeterogeneousAdoptionDiD.fit().
    Concrete fix: None required for this PR.

Security

No findings.

Documentation/Tests

  • Severity: P3
    Impact: Minor wording drift remains in result-field docs: SurveyDesign(psu=...) is used as shorthand even though HAD fit() rejects survey_design without a weights column, while the API guide correctly says to use SurveyDesign(weights='<weight_col>', psu='<cluster_col>'). See diff_diff/had.py:L366-L373, diff_diff/guides/llms-full.txt:L1558, diff_diff/guides/llms-full.txt:L1591, and the correct wording at docs/api/had.rst:L53-L56.
    Concrete fix: Reword those shorthand references to include the required weights= column.

Verification note: I could not run tests locally because this environment lacks pytest and numpy.

…rthand

A HAD survey design without a weights column is rejected, so the clustering
shorthand in the survey_binder_tsl_2sls result-field / llms-full descriptions
must be `SurveyDesign(weights='<weight_col>', psu='<cluster_col>')`, not a bare
`SurveyDesign(psu=...)` (matches the had.rst wording).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 4147cfad65eeac46d6a40b99cb9ea04c8502677f


Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • Affected method: HeterogeneousAdoptionDiD.fit() across static and event-study paths.
  • The removal of survey= / weights=, Binder-TSL-only weighting contract, and keyword-only cband are documented in docs/methodology/REGISTRY.md:L3186-L3187.
  • The implementation signature now exposes only survey_design= for weighting and makes cband keyword-only at diff_diff/had.py:L2973-L2985.
  • Inference fields still route through safe_inference() on static and event-study paths at diff_diff/had.py:L3774-L3783 and diff_diff/had.py:L4592-L4600.
  • Prior P3 wording drift around SurveyDesign(psu=...) appears resolved in the edited API/LLM docs.
  • One non-blocking stale-doc issue remains outside the edited doc files.

Methodology

  • Severity: P3 informational
    Impact: The API and variance-family change is a documented deviation/consolidation, not a methodology defect. The registry records survey_design= as the sole fit() weighting entry, removes pweight variance labels, and documents the mass-point vcov_type="classical" and cluster= + survey_design= fail-closed behavior at docs/methodology/REGISTRY.md:L3186-L3217.
    Concrete fix: None required.

Code Quality

No findings.

Performance

No findings. Removing dead pweight branches reduces branching surface.

Maintainability

No blocking findings. Remaining HAD pretest-helper alias cleanup is tracked in TODO.md:L137.

Tech Debt

  • Severity: P3 informational
    Impact: The seven remaining pretest-helper survey= / weights= aliases are deferred and explicitly tracked, so this is not a blocker.
    Concrete fix: None required for this PR.

Security

No findings.

Documentation/Tests

  • Severity: P2
    Impact: Public docs outside the edited files still describe the removed fit(weights=...) shortcut and old pweight / pweight_2sls labels, which now contradicts the new fit() API and can lead users to TypeError or stale inference expectations. See docs/choosing_estimator.rst:L695-L697 and docs/tutorials/20_had_brand_campaign.ipynb:L387-L391.
    Concrete fix: Update those passages to the new two-regime contract: unweighted vs survey_design=SurveyDesign(weights=...) Binder-TSL, with variance_formula limited to None, "survey_binder_tsl", and "survey_binder_tsl_2sls"; say survey= / weights= were removed on HeterogeneousAdoptionDiD.fit() in 3.7.0.

Verification note: I could not run pytest or import the package because this environment lacks numpy; I used source inspection and AST signature parsing.

…ng_estimator + tutorial 20

Two doc surfaces outside the core diff still described the removed
`HeterogeneousAdoptionDiD.fit(weights=)` shortcut and `pweight`/`pweight_2sls`
labels, made stale by the 3.7.0 removal:
- `choosing_estimator.rst`: rewrite the HAD SE-regime cell from three regimes
  (incl. the deprecated weights= shortcut + pweight labels) to the two-regime
  contract (unweighted vs `survey_design=` Binder-TSL; classical/cluster+survey
  fail-closed).
- `20_had_brand_campaign.ipynb`: the Extensions note said the `weights=`/`survey=`
  aliases "will be removed in the next minor release" -> now removed; point to
  `survey_design=`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: fc0652f7505b6216804016b5707b2408c9fa2fb1


Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • Affected method: HeterogeneousAdoptionDiD.fit() and its static/event-study survey-weighted paths.
  • The removal of survey= / weights=, Binder-TSL-only weighting contract, and keyword-only cband are documented in docs/methodology/REGISTRY.md:L3186-L3187.
  • The implementation signature now exposes only survey_design= for weighting and makes cband keyword-only at diff_diff/had.py:L2973-L2985.
  • Static and event-study inference still route through safe_inference() at diff_diff/had.py:L3780-L3783 and diff_diff/had.py:L4592-L4600.
  • The prior stale-doc issue appears resolved in docs/choosing_estimator.rst:L695-L697 and docs/tutorials/20_had_brand_campaign.ipynb:L387-L391.
  • Verification was limited: pytest is not installed in this environment, so I used source/AST inspection and git diff --check.

Methodology

  • Severity: P3 informational
    Impact: The API and variance-family consolidation is a documented deviation/contract change, not a methodology defect. The registry states that HeterogeneousAdoptionDiD.fit() removed survey= / weights=, keeps survey_design= as the sole weighting entry, limits variance_formula to None / "survey_binder_tsl" / "survey_binder_tsl_2sls", and documents the remaining mass-point/classical and cluster+survey fail-closed behavior.
    Concrete fix: None required.

Code Quality

No findings. The removed kwargs are absent from the public fit() signature, and tests assert TypeError for weights= / survey= plus keyword-only cband at tests/test_had_dual_knob_deprecation.py:L813-L837.

Performance

No findings. Removing dead pweight shortcut branches reduces branching and metadata construction work.

Maintainability

No findings. Remaining internal survey naming is private delegation into _fit_event_study, not public API drift.

Tech Debt

  • Severity: P3 informational
    Impact: The seven HAD pretest-helper alias removals remain deferred and tracked in TODO.md:L137, so they are not blockers under the deferred-work rules.
    Concrete fix: None required for this PR.

Security

No findings.

Documentation/Tests

No blocking findings. The previous stale references to removed fit(weights=...) / pweight variance labels were updated in the public estimator guide and tutorial, and the API docs now describe the two-regime contract at docs/api/had.rst:L43-L62. Tests cover removal, survey-design positive paths, cband behavior, and updated variance labels.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 7, 2026
@igerber igerber merged commit d6af5e1 into main Jul 7, 2026
35 of 36 checks passed
@igerber igerber deleted the refactor/had-drop-survey-weights-kwargs branch July 7, 2026 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant