feat(v4): semantic rename wave - post/control_group/vcov-only/level shims + missed-rename amendments (2(c)-ii PR-B, 15 rows) - #743
Conversation
…hims + missed-rename amendments (2(c)-ii PR-B, 15 rows)
Second and final 2(c)-ii rename-sweep PR (v4-design section 9 item 4).
Flips 12 ledger rows planned -> shimmed - M-030/M-031, M-043/M-095,
M-044/M-086/M-087, M-045..M-047/M-115, M-084 - and adds the three
missed-rename amendments M-136..M-138 (born shimmed; ledger + v4-design
section 3.4/8/9 edited in the same diff; EXPECTED_INITIAL_IDS and
ROW_COUNT_FLOOR raised to 117).
- time -> post (the 2x2 0/1 post dummy; rule 1): DifferenceInDifferences.fit
[M-030] and TripleDifference.fit [M-031] (whose warning notes time=
becomes calendar-only at 4.0 via M-085); the permutation_test /
leave_one_out_test diagnostics follow [M-137] [M-138] with docstrings
corrected from the wrong "Time period column". Calendar time= surfaces
(TWFE per M-082, CS, MPD, dCDH, SDID's own fit, CiC, ...) are untouched.
Internal migrations: the 7 AST-verified DiD.fit sites + the 4
newly-shimmed wrapper forwards in diagnostics.py (the
run_all_placebo_tests calls sit inside except-Exception blocks that
would swallow warnings-as-errors - pinned by explicit zero-warning +
non-error-dict tests), the DiD/DDD power fit-kwargs builders (TWFE
builders stay), the triple_difference() wrapper forward, and the
in-source emitters (practitioner/agent_workflow/datasets/prep_dgp
snippets). The run_placebo_test/run_all_placebo_tests wrappers keep
their genuinely OVERLOADED time (calendar for timing/group tests, post
dummy for the two renamed callees) - a rename cannot express dual
semantics; redesign tracked in TODO.md with honest guard allowlist
reasons.
- robust dropped everywhere it exists [M-045..M-047] [M-115]
(DiD + inherited TWFE/MPD, TripleDifference, HAD with its legacy False
default, LinearRegression): warn-and-route to the existing
resolve_vcov_type (now sentinel-aware with a legacy_default hook). The
PUBLIC .robust attr keeps the RESOLVED legacy bool through 3.9 (existing
attribute readers and default-construction pins keep passing) while
get_params returns the raw sentinel-era arg via _PARAM_ATTR_ALIASES +
_DERIVED_CONFIG_ATTRS, so clone round-trips stay silent; a
robust=-configured instance re-warns on set_params probe re-init
(documented). Internal forwards dropped (synthetic_did super().__init__,
sun_abraham's LinearRegression call).
- StackedDiD clean_control -> control_group [M-043]: lossless
sentinel+alias shim with _DERIVED_CONFIG_ATTRS ("control_group",) so
set_params updates the fit-time config, a _normalize_set_params hook
that lets either spelling migrate an instance in place, and a
deprecated warning-property for external attribute readers. Results
field clean_control -> control_group [M-095] (alias property,
__setstate__ pickle migration, dual to_dict keys through 3.9, summary
label "Control group:"). Consumers migrated per rule 11
(_reporting_helpers, business_report incl. the dual block keys and
narration prose, practitioner guidance); power.py's survey_config gate
is now ESTIMATOR-SCOPED so a default StackedDiD stays accepted
(bit-exact with the pre-rename gate, which rejected only strict).
- Wooldridge triple: aggregate type "event" -> "event_study" [M-086]
(aggregation_weights carries both keys through 3.9 + a __setstate__
mirror for old pickles); to_dataframe(aggregation=) ->
to_dataframe(level=) [M-044] with a sentinel default resolving to
"event_study"; summary(aggregation=) deprecated [M-087] - the
transitional summary(aggregation=SENTINEL, *, alpha=None) shape adopts
the library-wide section-5 contract (alpha != stored alpha RAISES; no
label-only relabeling), with the uniform positional summary(alpha=None)
arriving at 4.0. LPDiDResults.to_dataframe level "event" ->
"event_study" [M-136] (default flips; identical frame).
- ContinuousDiD covariates moves from __init__ to fit() [M-084]
(raw-keep ctor shim; both-supplied ValueError; the lowest_dose
NotImplementedError guard replicated for fit-level values).
tests/test_v4_rename_semantic.py pins all 15 rows (warning messages,
canonical/positional silence, both-supplied rejection, bit-exact routing
parity, field trio + pickle + dual keys, probe re-warn counts, the
wrapper zero-warning contract, and the power survey-gate regression).
~600 test call sites migrated to canonical names via a scope-aware
AST rewriter plus hand triage (deliberate alias-semantics tests keep
exercising the deprecated spellings, now wrapped in pytest.warns where
they construct outside warning-capture blocks); guides/rst/notebooks
swept with the CI-pinned llms-full signature blocks keeping annotated
deprecated tails; the ten touched tutorials execute end-to-end and
16_wooldridge_etwfe's stored outputs were refreshed in place. CHANGELOG
names the full row inventory.
Overall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
MethodologyNo defects found. Cross-checking against
Code QualityNo findings. The shared rename resolver and PerformanceNo findings. Changes add only constant-time argument resolution and compatibility bookkeeping. MaintainabilityNo findings. Raw versus resolved constructor state and bidirectional Tech Debt
SecurityNo findings. No secrets, credentials, unsafe evaluation, or new external-input execution paths were identified. Documentation/Tests
The new compatibility tests cover warnings, dual-name rejection, positional stability, routing parity, pickle migration, and parameter round-trips by inspection. |
…v_type=/fit-covariates/event_study guidance
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
MethodologyNo findings. Affected methods were checked against their Registry entries: DiD, TripleDifference, ContinuousDiD, StackedDiD, Wooldridge ETWFE, LPDiD result formatting, and placebo diagnostics ( The inline permutation t-stat remains the documented randomization-inference exception at Code QualityNo findings. Deprecated aliases consistently reject dual spellings and resolve to canonical body-local values. PerformanceNo findings. The shims add only constant-time argument and compatibility handling. MaintainabilityNo findings. Keeping Tech Debt
SecurityNo findings. No secrets, unsafe evaluation, or new external-input execution paths were identified. Documentation/TestsNo findings. The previous documentation issue is resolved: deprecated AST syntax validation passed for all 60 changed Python/test files inspected. Runtime tests could not be executed because pytest is not installed. |
…trol_group (M-095) The two failing legs' tests build minimal stub result objects and were still setting the pre-rename clean_control attribute; the M-095 field rename made describe_target_parameter read control_group (no fallback - __setstate__ guarantees the field on real results), so the stubs fell through to the default not-yet-treated clause. Stubs, parametrize name, and docstring migrated to the canonical name.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
MethodologyNo actionable findings. Affected methods were cross-checked against the Registry: DifferenceInDifferences, ContinuousDiD, StackedDiD, Wooldridge ETWFE, LPDiD, TripleDifference, and placebo diagnostics (
Code QualityNo findings. Canonical StackedDiD reporting now consistently reads PerformanceNo findings. Compatibility shims add only constant-time argument handling. MaintainabilityNo findings. The re-review delta consistently replaces obsolete test-stub fields across all control-group branches. Tech Debt
SecurityNo findings. No new unsafe execution paths, secrets, or sensitive data were identified. Documentation/TestsNo findings. The follow-up commit correctly exercises |
Summary
planned→shimmed— [M-030]/[M-031] (time=→post=on the 2x2/2x2x2 post dummy), [M-043]/[M-095] (clean_control→control_group, estimator + results field), [M-044]/[M-086]/[M-087] (the Wooldridgelevel/event_study/summarytriple), [M-045]..[M-047]/[M-115] (therobustdrop, all four sites), [M-084] (ContinuousDiDcovariatesmoves tofit()) — and adds three missed-rename amendment rows [M-136]..[M-138] (LPDiDlevel="event"value; the two post-dummy diagnostics params), bornshimmedwith ledger + v4-design §3.4/§8/§9 edited in the same diff (EXPECTED_INITIAL_IDS/ROW_COUNT_FLOOR→ 117)._deprecation.pymachinery: singleFutureWarningwith the uniform migration message, both-spellingsValueError, positional binding preserved, deprecated paths route to bit-identical numbers. M-031's warning notestime=becomes calendar-only at 4.0 ([M-085]).robustkeeps the RESOLVED legacy bool on the public attr whileget_params()returns the raw sentinel-era arg (silent clone round-trips; configured instances re-warn on probe re-init, documented); StackedDiD's lossless shim carries_DERIVED_CONFIG_ATTRS+ a both-directions_normalize_set_paramshook + a deprecated estimator-attr property (guard-allowlisted)._reporting_helpers/business_reportread the renamed field with dual serialized keys through 3.9; power.py's survey gate is now estimator-scoped so a default StackedDiD stays accepted (bit-exact with the pre-rename gate); the four newly-shimmed diagnostics forwards migrated (theexcept Exceptionwrapper paths pinned zero-warning + non-error-dict); DiD/DDD power builders migrated while TWFE builders stay ontime=([M-082] is phase 3).run_placebo_test/run_all_placebo_testswrappers keep their genuinely OVERLOADEDtime(calendar for timing/group tests, post dummy for the renamed callees) — a rename cannot express dual semantics; the signature redesign is a TODO.md row with honest guard-allowlist reasons.WooldridgeDiDResults.summaryadopts the library-wide §5 contract:alpha(keyword-only during 3.9) RAISES on a value different from the stored one — no label-only relabeling of stored intervals.Methodology references (required if estimator / math changes)
docs/v4-design.md§8 (rule 1postvs calendartime, rule 6control_group, §7'srobustretirement, rule 10 public-function completeness, the missed-rename clause for [M-136]..[M-138]).time=surfaces (TWFE, CS, MPD, dCDH, SDID's own fit, CiC/QDiD) are deliberately untouched; CS-family cohort vocabulary unchanged.Validation
tests/test_v4_rename_semantic.py(47 pins incl. warning contracts, routing parity, field trio + pickle + dual keys, probe re-warn counts, wrapper zero-warning assertions, the power survey-gate regression, and the summary-alpha reject contract); ~600 test call sites migrated to canonical names (scope-aware AST rewriter + hand triage; deliberate alias-semantics tests keep exercising deprecated spellings underpytest.warns);tests/test_v4_matrix.py+tests/test_naming_guard.pygreen with all 15 rows and the amendment bookkeeping.16_wooldridge_etwfe's stored outputs re-executed in place after its display calls changed. Numbers unchanged (rename-only routing).Security / privacy