Skip to content

Commit 3da2f0f

Browse files
authored
feat(v4): PR 3b of the 4.0 cut - remove the fit-time aggregate=/balance_e= keywords on CS, EfficientDiD, StackedDiD, dCDH and HAD (#864)
1 parent 13155a9 commit 3da2f0f

57 files changed

Lines changed: 782 additions & 1304 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎benchmarks/python/benchmark_callaway.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ def main():
124124
)
125125
# Post-fit aggregations stay inside the timed region: the R `aggte`
126126
# yardstick is fit + simple/dynamic/group aggregation (the 3.x
127-
# fit(aggregate="all") route, deprecated since 3.9 and removed at the 4.0 cut).
127+
# fit(aggregate="all") route, removed at 4.0).
128128
es = results.aggregate("event_study")
129129
grp = results.aggregate("group")
130130

‎benchmarks/python/benchmark_stacked_did.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ def main():
9696
)
9797

9898
# The event-study surface is always computed at fit (3.9, row M-024);
99-
# the deprecated aggregate= kwarg would emit a FutureWarning inside
100-
# the timed block and contaminate the measurement.
99+
# fit() takes no aggregate= kwarg since 4.0.
101100
with Timer() as estimation_timer:
102101
results = est.fit(
103102
df,

‎benchmarks/speed_review/bench_callaway.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def bench_fit(n_units, n_bootstrap=0, covariates=None, n_cohorts=5, n_runs=3,
6666
covariates=covariates,
6767
)
6868
# Keep "fit + aggregations" as the measured work (the 3.x
69-
# fit(aggregate="all") route, deprecated since 3.9 and removed at the 4.0 cut).
69+
# fit(aggregate="all") route, removed at 4.0).
7070
res.aggregate("event_study")
7171
res.aggregate("group")
7272
elapsed = time.perf_counter() - start

‎benchmarks/speed_review/bench_solve_ols_fastpath.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def fit(d):
126126
covariates=covs,
127127
)
128128
# fit + aggregations is the measured work (the 3.x
129-
# fit(aggregate="all") route, deprecated since 3.9 and removed at the 4.0 cut).
129+
# fit(aggregate="all") route, removed at 4.0).
130130
res.aggregate("event_study")
131131
res.aggregate("group")
132132
return float(res.overall_att), float(res.overall_se)

‎benchmarks/speed_review/validate_results.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ def run_estimator(df, estimation_method="reg", covariates=None, control_group="n
6565
first_treat="first_treat",
6666
covariates=covariates,
6767
)
68-
# Post-fit containers (the 3.x fit(aggregate="all") route is deprecated and goes at the 4.0 cut);
68+
# Post-fit containers (the 3.x fit(aggregate="all") route, removed at 4.0);
6969
# the fit is bootstrapped, so each call replays the fit-time bootstrap.
7070
es_container = results.aggregate("event_study")
7171
grp_container = results.aggregate("group")
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
### Removed
2+
- **The fit-time `aggregate=` / `balance_e=` keywords are gone on the estimators whose post-fit
3+
route already existed (ledger rows flipped to `removed`).** `fit(aggregate=)` is removed from
4+
`CallawaySantAnna` (`M-020`), `EfficientDiD` (`M-023`), `StackedDiD` (`M-024`),
5+
`ChaisemartinDHaultfoeuille` (`M-026`) and `HeterogeneousAdoptionDiD` (`M-027`),
6+
`fit(balance_e=)` from `CallawaySantAnna` (`M-117`) and `EfficientDiD` (`M-120`), and
7+
`aggregate=` from `did_had_pretest_workflow` (`M-139`; the `HADPretestReport.aggregate`
8+
output field survives). The successor is post-fit `results.aggregate(type, balance_e=)`; HAD
9+
selects its mode from the panel shape. With the keywords go StackedDiD's fit-time
10+
`group`/`all`/unknown-value `ValueError`s (`aggregate('group')` already fails closed in the
11+
mixin) and the private `_VALID_AGGREGATES` sets in `had.py` / `had_pretests.py`
12+
(`_infer_aggregate_mode` is the sole selector). `ImputationDiD`, `TwoStageDiD` and
13+
`ContinuousDiD` keep their deprecated keywords until the next cut PR adds the
14+
bootstrapped-fit relay.
15+
- **CallawaySantAnna and EfficientDiD compute no event-study/group surface at fit.** Their
16+
legacy `event_study_effects` / `group_effects` fields - and CS's `cband_crit_value`,
17+
`event_study_vcov`, `event_study_vcov_index`, `event_study_df` - are `None` after EVERY fit;
18+
the post-fit replay (`aggregate('event_study'|'group', balance_e=)`) is the only bootstrap
19+
route and is unchanged.
20+
21+
### Breaking Changes
22+
- Passing `aggregate=` or `balance_e=` to `CallawaySantAnna.fit`, `EfficientDiD.fit`,
23+
`StackedDiD.fit`, `ChaisemartinDHaultfoeuille.fit` or `HeterogeneousAdoptionDiD.fit` (or
24+
`aggregate=` to `did_had_pretest_workflow`) now raises Python's native
25+
`TypeError: ... unexpected keyword argument`. `TripleDifference`'s staggered-mode
26+
`aggregate=`/`balance_e=` are untouched (`M-140`/`M-141`, canonical until the DDD container
27+
unification).
28+
- **Positional slots after the removed parameters shift left.** `CallawaySantAnna`:
29+
`survey_design` 9 -> 7; `EfficientDiD`: `survey_design` 9 -> 7, `store_eif` 10 -> 8;
30+
`StackedDiD` (`aggregate` sat before them): `population`, `survey_design`, `covariates`
31+
7/8/9 -> 6/7/8; `ChaisemartinDHaultfoeuille`: `L_max`, `covariates`, `trends_linear`,
32+
`trends_nonparam`, `honest_did`, `heterogeneity`, `design2`, `survey_design` each move one
33+
slot left (7..14 -> 6..13). `HeterogeneousAdoptionDiD.fit` shifts nothing (everything after
34+
the removed slot is keyword-only; a seventh positional now raises). Pass these by keyword.
35+
- `CallawaySantAnnaResults.to_dataframe(level="event_study"|"group")` and the
36+
`EfficientDiDResults` twins raise on every fresh fit (the legacy fields are always `None`);
37+
use `results.aggregate(...).to_dataframe()`. The column-schema rewire of these levels onto the
38+
container is `M-093`'s (PR 8).
39+
40+
### Changed
41+
- Runtime messages, the four `llms*.txt` guides, the API docs, the migration guide and the
42+
benchmarks no longer steer users to the fit-time route on these five estimators;
43+
`CallawaySantAnna`'s class docstring, `plot_event_study`, `honest_did` / `pretrends` and the
44+
results-class "not stored" messages point at `results.aggregate('event_study')`.
45+
46+
### Internal
47+
- `tests/test_aggregate_contract.py`: the `Test*Shim` classes, fit-time fixtures and
48+
inertness comparisons for the five estimators are retired in favour of `Test*Removal`
49+
pins (TypeError, signature, positional-slot binding); the committed oracle fixture is the
50+
numerical reference. `tests/_capture_v4_aggregate_oracles.py` is marked historical (it
51+
drives the removed route; runnable only at the recorded `captured_at_head`). Naming-guard
52+
allowlist entries for the `llms.txt` / `llms-full.txt` canonical post-fit `aggregate`
53+
vocabulary. Ledger row `M-093` records that CS/EfficientDiD populate the legacy fields on
54+
no fit. `warn_deprecated_kwarg` loses its last library caller and is retained (spec section
55+
8 rule 11) with a direct unit pin.

‎diff_diff/_deprecation.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,14 @@
1616
default. ``resolve_renamed_kwarg`` maps old to new with a single
1717
``FutureWarning``, rejects both-supplied calls loudly, and returns the
1818
caller-declared default when neither is passed.
19-
- Dropped parameter (no successor name, e.g. the fit-time ``aggregate=``
20-
override): the parameter keeps its position with a sentinel default; the
21-
owner calls ``warn_deprecated_kwarg`` when a value was actually supplied.
19+
- Dropped parameter (no successor name): the parameter keeps its position
20+
with a sentinel default; the owner calls ``warn_deprecated_kwarg`` when a
21+
value was actually supplied. (The 3.9 fit-time ``aggregate=`` shims on
22+
CallawaySantAnna, EfficientDiD, StackedDiD, ChaisemartinDHaultfoeuille and
23+
HeterogeneousAdoptionDiD were this shape; they were deleted at the 4.0 cut
24+
(PR 3b) and ImputationDiD / TwoStageDiD / ContinuousDiD follow in PR 3c.
25+
``warn_deprecated_kwarg`` has no library caller any more (dCDH was the
26+
last) and is retained per section 8 rule 11 for the 5.0 window.)
2227
- Renamed results field: the dataclass field takes the new name and
2328
``deprecated_field_property`` builds the read-only warning alias under the
2429
old name (a plain class attribute, so it is a descriptor but NOT a

‎diff_diff/_reporting_helpers.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,11 @@ def describe_target_parameter(results: Any) -> Dict[str, Any]:
5656
disambiguate via the event-study or group aggregate if you need
5757
the horizon / group target.
5858
- ``CallawaySantAnna``: ``overall_att`` is cohort-size-weighted
59-
across post-treatment ``ATT(g, t)`` cells regardless of the
60-
fit-time ``aggregate`` kwarg. The event-study / group tables are
61-
produced post-fit via ``results.aggregate('event_study'/'group')``
62-
(the deprecated fit-time ``aggregate=`` kwarg populates the legacy
63-
``event_study_effects`` / ``group_effects`` fields until 4.0).
59+
across post-treatment ``ATT(g, t)`` cells regardless of any
60+
aggregation. The event-study / group tables are produced post-fit
61+
via ``results.aggregate('event_study'/'group')``; the legacy
62+
``event_study_effects`` / ``group_effects`` fields are None on
63+
every CallawaySantAnna fit since 4.0.
6464
- ``ContinuousDiD``: the regime (PT vs. SPT) is a user-level
6565
assumption, not a library setting. The ``definition`` names
6666
both regime readings (``ATT^loc`` under PT,
@@ -140,9 +140,9 @@ def describe_target_parameter(results: Any) -> Dict[str, Any]:
140140
"``overall_att`` is the simple-aggregation headline regardless "
141141
"of aggregation choices; event-study and group tables are "
142142
"produced post-fit via "
143-
"``results.aggregate('event_study'/'group')`` (the deprecated "
144-
"fit-time ``aggregate=`` kwarg populates the legacy "
145-
"``event_study_effects`` / ``group_effects`` fields until 4.0)."
143+
"``results.aggregate('event_study'/'group')`` (the legacy "
144+
"``event_study_effects`` / ``group_effects`` fields are None "
145+
"on every CallawaySantAnna fit since 4.0)."
146146
),
147147
"aggregation": "simple",
148148
"headline_attribute": "overall_att",

‎diff_diff/chaisemartin_dhaultfoeuille.py‎

Lines changed: 6 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import pandas as pd
3838

3939
from diff_diff._base import BaseEstimator
40-
from diff_diff._deprecation import NOT_SUPPLIED, warn_deprecated_kwarg
4140
from diff_diff.chaisemartin_dhaultfoeuille_bootstrap import (
4241
ChaisemartinDHaultfoeuilleBootstrapMixin,
4342
)
@@ -401,8 +400,8 @@ class ChaisemartinDHaultfoeuille(ChaisemartinDHaultfoeuilleBootstrapMixin, BaseE
401400
- TWFE decomposition diagnostic from Theorem 1 of AER 2020
402401
403402
Aggregation is a post-fit step: ``results.aggregate('event_study')`` /
404-
``results.aggregate('simple')`` (the deprecated fit-time ``aggregate=``
405-
never computed anything and now warns; row M-026).
403+
``results.aggregate('simple')``; ``fit()`` takes no aggregation
404+
keyword.
406405
407406
Parameters
408407
----------
@@ -878,7 +877,6 @@ def fit(
878877
time: str,
879878
treatment: str,
880879
# ---------- forward-compat parameters ----------
881-
aggregate: Any = NOT_SUPPLIED,
882880
L_max: Optional[int] = None,
883881
covariates: Optional[List[str]] = None,
884882
trends_linear: Optional[bool] = None,
@@ -914,15 +912,6 @@ def fit(
914912
within each ``(unit, time)`` cell. Both binary ``{0, 1}`` and
915913
non-binary (ordinal or continuous) treatment are supported.
916914
Non-binary treatment requires ``L_max >= 1``.
917-
aggregate : str, optional
918-
DEPRECATED since 3.9, removed in 4.0 (ledger row M-026).
919-
Passing it emits a ``FutureWarning``. The parameter never
920-
computed aggregations here (it was reserved and any non-None
921-
value has always raised); aggregation is a POST-FIT step:
922-
``results.aggregate('event_study')`` for the multi-horizon
923-
surface, ``results.aggregate('simple')`` for the overall
924-
estimand. A non-None value now raises ``ValueError`` pointing
925-
at that post-fit route.
926915
L_max : int, optional
927916
Maximum event-study horizon. When set, computes ``DID_l``
928917
for ``l = 1, ..., L_max`` using the per-group building block
@@ -1063,28 +1052,8 @@ def fit(
10631052
# ------------------------------------------------------------------
10641053
# Step 2: Forward-compat gates
10651054
# ------------------------------------------------------------------
1066-
# M-026: fit(aggregate=) is deprecated - the aggregation entry point
1067-
# is post-fit (results.aggregate()). The param never computed
1068-
# anything here (any non-None value has always raised), so the shim
1069-
# warns on supply and rejects non-None values with a pointer at the
1070-
# post-fit route.
1071-
if aggregate is not NOT_SUPPLIED:
1072-
warn_deprecated_kwarg(
1073-
"ChaisemartinDHaultfoeuille.fit",
1074-
"aggregate",
1075-
"aggregation is a post-fit step: "
1076-
"results.aggregate('event_study') / results.aggregate('simple')",
1077-
)
1078-
if aggregate is not None:
1079-
raise ValueError(
1080-
f"fit(aggregate={aggregate!r}) never computed aggregations "
1081-
"on ChaisemartinDHaultfoeuille (the parameter was reserved "
1082-
"and always raised). Aggregate post-fit instead: "
1083-
"results.aggregate('event_study') for the multi-horizon "
1084-
"surface (Phase-1 fits without L_max return the 2-row "
1085-
"l=1 view), or results.aggregate('simple') for the "
1086-
"overall estimand."
1087-
)
1055+
# Aggregation is a post-fit step (results.aggregate('event_study') /
1056+
# results.aggregate('simple')); fit() takes no aggregation keyword.
10881057
_check_forward_compat_gates(
10891058
L_max=L_max,
10901059
controls=controls,
@@ -4395,10 +4364,8 @@ def _check_forward_compat_gates(
43954364
"""Raise ``NotImplementedError`` for any non-default Phase 3 parameter.
43964365
43974366
Phase 2 parameters (``L_max``) are validated inline in ``fit()``
4398-
after period detection. The deprecated ``aggregate`` parameter is
4399-
resolved in ``fit()`` itself (row M-026: FutureWarning on supply,
4400-
ValueError on any non-None value, pointing at the post-fit
4401-
``results.aggregate()`` route).
4367+
after period detection. Aggregation is a post-fit step
4368+
(``results.aggregate()``); ``fit()`` takes no aggregation keyword.
44024369
"""
44034370
# L_max is validated inline in fit() after period detection (needs
44044371
# the period count). Not gated here.

‎diff_diff/diagnostic_report.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ def _resolve_event_study_surface(
807807
808808
The raw ``event_study_effects`` field always takes precedence when it
809809
is not ``None`` — including the requested-but-empty ``{}`` sentinel,
810-
which encodes fit-time configuration (e.g. an EfficientDiD
810+
which encodes fit-time configuration (e.g. an ImputationDiD
811811
``balance_e=`` that emptied the window) that a re-derivation without
812812
those settings would silently overturn. Derivation failures
813813
(bootstrapped fits, missing kits) are cached as skip reasons, never
@@ -1006,9 +1006,9 @@ def _instance_skip_reason(self, check: str) -> Optional[str]:
10061006
"per-event-time direct-effect output."
10071007
)
10081008
# StackedDiD (3.9, row M-024): the event-study surface
1009-
# is ALWAYS computed, so ``aggregate='event_study'`` is
1010-
# inert (and deprecated - it now warns). Zero estimated
1011-
# pre-periods here means the event window has none:
1009+
# is ALWAYS computed at fit (no aggregation keyword
1010+
# exists since 4.0). Zero estimated pre-periods here
1011+
# means the event window has none:
10121012
# the default ``kappa_pre=1`` grid minus the reference
10131013
# leaves no pre-period columns. Estimator-accurate
10141014
# remediation, per the SpilloverDiD precedent above.

0 commit comments

Comments
 (0)