refactor(had): remove deprecated survey=/weights= kwargs from the 7 pretest helpers (3.7.x)#659
refactor(had): remove deprecated survey=/weights= kwargs from the 7 pretest helpers (3.7.x)#659igerber wants to merge 8 commits into
Conversation
…retest helpers (3.7.x) Completes the HAD survey-design API consolidation started with HeterogeneousAdoptionDiD.fit in 3.7.0: did_had_pretest_workflow, qug_test, stute_test, yatchew_hr_test, stute_joint_pretest, joint_pretrends_test, and joint_homogeneity_test now accept survey_design= only — the old aliases raise TypeError. Per-helper tracing (not mechanical deletion): array-in helpers keep their internal survey/weights rebind with the alias shims gone; data-in helpers lose the row-level weights= array shortcut (migration = add a weight column + SurveyDesign(weights='col'); per-unit aggregation + mean-1 normalization made the two forms numerically identical, locked previously). Dead code removed with the aliases: 3-way alias mutexes, the five deprecation/mutex message constants in survey.py, the workflow's row-level weight alignment + DeprecationWarning-suppression forwarding machinery, and the joint wrappers' staggered weight-subsetting blocks (SurveyDesign column references self-align). qug_test still permanently rejects survey_design= (Phase 4.5 C0); its NotImplementedError text updated. Surviving survey_design= / unweighted paths byte-identical (no computational code touched; 545 HAD tests pass with only alias-touching tests rewritten: rejection tests + column-form conversions + 4 obsolete length-validation tests for the removed array path deleted + the weights=/survey= equivalence lock repurposed as a column-name-invariance lock). Docs swept: REGISTRY (4 passages), llms-full, had.rst, tutorial 21 prose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
igerber
force-pushed
the
refactor/had-pretest-drop-survey-weights-kwargs
branch
from
ead3e5a to
f6b7d1c
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologyNo P0/P1 findings. The QUG survey rejection remains consistent with the registry’s Phase 4.5 C0 deferral, and the Stute/Yatchew survey algorithms remain aligned with the documented Phase 4.5 C mechanisms. The alias removal is explicitly documented in Code QualityNo blocking findings. P3 — stale comments/dead branch wording PerformanceNo findings. The PR removes alias-handling code and does not introduce new computation. MaintainabilityNo P1/P2 findings. P3 — obsolete helper still carries removed public API language Tech DebtNo blocking findings. The removed TODO row for HAD pretest alias removal matches the implemented API consolidation. SecurityNo findings. No secrets or security-sensitive code paths observed in the diff. Documentation/TestsP3 — stale public-contract wording after alias removal Tests not run: importing the package in this environment failed because |
igerber
added
ready-for-ci
CI caught tests/test_had_dual_knob_deprecation.py - a dedicated alias-surface test file my sweep missed (42 failures: deprecation warnings, 3-way mutexes, alias type-guards, alias parity, all exercising the removed kwargs). Rewritten to the removal contract: per-surface weights=/survey= TypeError pins (mirroring the existing TestHADFitDeprecation style), alias-parity/mutex/via-alias tests deleted as obsolete, the scale-invariance locks converted to the canonical make_pweight_design entry (same invariant, same tolerances), module docstring updated. 39 pass. Review P3s: dead "elif weights is not None" validation branches excised from stute_test / yatchew_hr_test / stute_joint_pretest (unreachable after the rebind); trends_lin gates simplified; _resolve_pretest_unit_weights re-documented (weights param = internal legacy plumbing, mutex message no longer names public kwargs); stale survey/weights prose fixed in qug docs, the workflow warning, REGISTRY C0 notes, and test docstrings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologyNo P0/P1 findings. The registry now documents the removed Code QualityP3 — stale internal wording after alias removal PerformanceNo findings. The PR removes alias plumbing and does not add new computation. MaintainabilityP3 — retained dead-ish internal Tech DebtNo blocking findings. The TODO row for pretest alias removal was removed, and the remaining QUG survey deferral is still tracked at SecurityNo findings. No secrets or security-sensitive code paths observed. Documentation/TestsP3 — contradictory Unreleased changelog note P3 — stale test prose/comments Verification: AST parse passed for the changed Python files. Tests were not run because |
…I-rerun P3s) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologyNo findings. The registry documents the canonical Code QualityP3 - residual stale internal wording PerformanceNo findings. MaintainabilityNo findings. The retained Tech DebtNo blocking findings. The completed pretest-alias TODO row was removed, and the QUG survey-design limitation remains tracked at SecurityNo findings. No secrets or security-sensitive changes observed. Documentation/TestsP3 - stale line references / test prose Verification: AST parsing passed for |
…(CI-rerun round-3 P3s) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologyNo findings. The registry documents Code QualityNo findings. The removal is consistently propagated through the public signatures and internal forwarding paths. Data-in surfaces now forward PerformanceNo findings. Alias-removal deletes branches and warning/mutex plumbing; no new computational path or extra pass over data was added. MaintainabilityNo findings. The retained Tech DebtNo blocking findings. The completed pretest-alias TODO row was removed, and the remaining QUG survey-design limitation stays tracked in SecurityNo findings. No secrets or security-sensitive changes observed. Documentation/TestsP3 - stale test prose only Verification: |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologyNo findings. The registry explicitly documents the 3.7.x removal and the surviving survey methodology: Code QualityNo findings. Removed constants have no remaining references, and public callers using removed aliases are limited to intentional TypeError tests. PerformanceNo findings. This removes alias/mutex/deprecation branches and does not add new computational passes. MaintainabilityP3 - stale comments only Tech DebtNo findings. The completed pretest-helper alias TODO was removed, while the QUG survey-design limitation remains tracked at SecurityNo findings. No secrets or security-sensitive changes observed. Documentation/TestsP3 - guide inconsistency on stratified pretests Verification: AST parsing passed for |
…t sentence Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologyP3 - documented API/methodology surface change Code QualityP2 - unused PerformanceNo findings. MaintainabilityNo findings beyond the unused-local cleanup above. Tech DebtNo findings. The completed pretest-helper alias TODO was removed, and the remaining QUG survey-design limitation is still tracked at SecurityNo findings. Documentation/TestsP3 - stale test prose Verification: direct syntax compilation passed for |
…tring (P3) AST-verified: all five weights locals were stores with zero loads after the dead-branch excisions; comments now name only the surviving survey internal. Section header + TestQUGTestDeprecation docstring distinguish the permanent survey_design= NotImplementedError gate from the removed-alias TypeError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologySeverity: P3 - documented API surface change Code QualityNo findings. PerformanceNo findings. MaintainabilityNo findings. The retained internal Tech DebtNo findings. The completed pretest-helper alias TODO was removed, and the remaining QUG survey-design limitation is still tracked at SecurityNo findings. Documentation/TestsNo blocking findings. Docs and tests now consistently describe the removed aliases and migration path at Verification performed:
|
The t22 drift-guard file pins the exact report.summary() skip-note string, which the round-2 prose sweep reworded (survey/weights -> survey designs). Sync every consumer the guard protects: the pinned constant + module/test docstrings, tutorial 21/22 markdown prose, and survey-roadmap.md. Repo-wide grep now finds zero old-string consumers; FULL suite run locally: 8563 passed, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
MethodologySeverity: P3 - documented API-surface removal Severity: P3 - documented QUG survey deferral Code QualityNo findings. The retained PerformanceNo findings. The PR removes alias/mutex/deprecation plumbing and does not introduce new loops or heavier computation on the surviving paths. MaintainabilityNo findings. The public surface is simpler, and the remaining canonical Tech DebtNo blocking findings. The completed pretest-helper alias TODO row was removed, and the still-deferred survey-aware QUG item remains tracked. Relevant location: SecurityNo findings. Documentation/TestsSeverity: P3 - stale/ambiguous tutorial wording Verification performed:
|
1 participant
Summary
HeterogeneousAdoptionDiD.fitin 3.7.0 (refactor(had): remove deprecated survey=/weights= kwargs from HeterogeneousAdoptionDiD.fit (3.7.0) #633): all 7 pretest surfaces —did_had_pretest_workflow,qug_test,stute_test,yatchew_hr_test,stute_joint_pretest,joint_pretrends_test,joint_homogeneity_test— now acceptsurvey_design=only; passing the oldsurvey=/weights=aliases raisesTypeError. Per-helper tracing as the TODO row required, not mechanical deletion: array-in helpers keep their internalsurvey/weightsvariable rebind with the alias shims gone; data-in surfaces lose the row-levelweights=array shortcut entirely (migration: add a weight column +SurveyDesign(weights='col')— per-unit aggregation + mean-1 normalization made the two forms numerically identical, a lock the old equivalence test already proved; it's repurposed as a column-name-invariance lock).survey.py, the workflow's row-level weight alignment +DeprecationWarning-suppression forwarding machinery (which existed solely to carry the removed kwarg downstream), and the joint wrappers' staggered weight-subsetting blocks (SurveyDesigncolumn references self-align through filtering).qug_teststill permanently rejectssurvey_design=(Phase 4.5 C0) with updated message text. Net −465 LoC.llms-full.txt,docs/api/had.rst, tutorial 21 prose — zero stalesurvey=/weights=mentions remain on the pretest surfaces.Methodology references (required if estimator / math changes)
Validation
tests/test_had_pretests.py(rejection tests + canonical-form conversions; 241 pass),tests/test_had.pyunchanged (302 pass),tests/test_survey.pyunchanged.Security / privacy
Generated with Claude Code