Skip to content

Commit bcada5c

Browse files
igerberclaude
andcommitted
Codex re-review round 3: TWFE contract narrowing + lag_cutoff propagation
P1 (Methodology): the prior commit's "arbitrary ordered labels" contract was unreachable on TwoWayFixedEffects because TWFE's design step builds `_treatment_post = treated * time` from raw column values, which fails on datetime64 / pd.Period / string labels. Narrowing the docs to make explicit that the non-numeric-label contract is MultiPeriodDiD-only (MPD builds period dummies, not a `treated * time` product). TWFE inherits its pre-existing numeric-time constraint. - llms-full.txt: split the panel example into a TWFE block (binary post indicator only, `conley_lag_cutoff=1`) and an MPD block (multi-period time, arbitrary orderable encoding). New caveat paragraph spells out the TWFE numeric-time constraint. - test_twfe_conley_non_numeric_time_fails: TWFE + string-encoded time raises a clean error (string * int multiplication fails inside the estimator) — regression for the narrowed contract. P1 (Maintainability): `conley_lag_cutoff` is a new public parameter that materially changes vcov semantics (0 = spatial only, >0 = adds within- unit Bartlett serial HAC), but the result objects didn't expose it. - DiDResults + MultiPeriodDiDResults: new `conley_lag_cutoff: Optional[int]` field threaded through the estimator-side construction. - `_format_vcov_label` now includes `lag_cutoff=<int>` in the Conley label so summary() readers can tell which Conley variant produced the reported SEs (e.g. "Conley spatial HAC (1999), lag_cutoff=1"). - TestConleyTWFE summary test asserts both the label format and the programmatic `res.conley_lag_cutoff` accessor. P3 (Doc/test): MPD docstring claimed `inference="wild_bootstrap"` + conley raises but only TWFE had that guard. Added the explicit raise in MPD.fit() to match the documented contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3755a44 commit bcada5c

5 files changed

Lines changed: 86 additions & 28 deletions

File tree

diff_diff/estimators.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,7 @@ def _refit_did_absorb(w_r):
601601
# stored `self.vcov_type`.
602602
vcov_type=_fit_vcov_type,
603603
cluster_name=self.cluster,
604+
conley_lag_cutoff=(self.conley_lag_cutoff if _fit_vcov_type == "conley" else None),
604605
)
605606

606607
self._coefficients = coefficients
@@ -1416,6 +1417,14 @@ def fit( # type: ignore[override]
14161417
"cluster-robust without spatial HAC, or drop survey_design= "
14171418
"for panel Conley."
14181419
)
1420+
if self.inference == "wild_bootstrap":
1421+
raise NotImplementedError(
1422+
"MultiPeriodDiD(vcov_type='conley', "
1423+
"inference='wild_bootstrap') is not supported: wild "
1424+
"bootstrap is a separate inference path that does not "
1425+
"consume the analytical Conley sandwich. Use "
1426+
"inference='analytical' for Conley SEs."
1427+
)
14191428
# Pre-compute non_ref_periods (needed for absorb demeaning)
14201429
non_ref_periods = [p for p in all_periods if p != reference_period]
14211430

@@ -1878,6 +1887,7 @@ def _refit_mp_absorb(w_r):
18781887
n_clusters=(
18791888
len(np.unique(effective_cluster_ids)) if effective_cluster_ids is not None else None
18801889
),
1890+
conley_lag_cutoff=(self.conley_lag_cutoff if _fit_vcov_type == "conley" else None),
18811891
)
18821892

18831893
self._coefficients = coefficients

diff_diff/guides/llms-full.txt

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1924,39 +1924,47 @@ reg = LinearRegression(
19241924
).fit(X, y)
19251925
se = np.sqrt(np.diag(reg.vcov_))
19261926

1927-
# Panel design: TWFE with within-unit Bartlett serial HAC (lag = 2 periods).
1928-
# `time` must be a panel-period index column with one row per (unit, period).
1929-
# diff-diff internally normalizes the time labels to dense panel-period codes
1930-
# 0..T-1, so any orderable encoding works — int years (2020, 2021, ...),
1931-
# YYYYMM (202012, 202101, ...), datetime64, pd.Period, strings.
1927+
# Panel design: TWFE with within-unit Bartlett serial HAC.
1928+
# TWFE's `time` column is intrinsically a binary post indicator
1929+
# (treatment * time interaction); only numeric encodings are supported on
1930+
# this surface. `conley_lag_cutoff=1` includes the cross-period pair under
1931+
# the Bartlett taper. For multi-period panels, use MultiPeriodDiD.
19321932
res = TwoWayFixedEffects(
19331933
vcov_type="conley",
19341934
conley_coords=("lat", "lon"), # column names on `data`
19351935
conley_cutoff_km=500.0,
1936-
conley_lag_cutoff=2, # within-unit Bartlett up to 2 panel periods
1937-
).fit(data, outcome="y", treatment="treated", time="period", unit="unit_id")
1936+
conley_lag_cutoff=1, # within-unit Bartlett, lag 1 panel period
1937+
).fit(data, outcome="y", treatment="treated", time="post", unit="unit_id")
19381938

1939-
# Panel design: MultiPeriodDiD with cross-sectional within-period only (lag=0)
1939+
# Panel design: MultiPeriodDiD with multi-period time.
1940+
# MultiPeriodDiD builds period dummies (NOT a treated * time product), so
1941+
# `time` can be any orderable encoding — int years (2020, 2021, ...),
1942+
# YYYYMM (202012, 202101, ...), datetime64, pd.Period, strings.
1943+
# `_compute_conley_vcov` normalizes time to dense codes 0..T-1 internally,
1944+
# so `conley_lag_cutoff` always counts panel periods regardless of label.
19401945
mp_res = MultiPeriodDiD(
19411946
vcov_type="conley",
19421947
conley_coords=("lat", "lon"),
19431948
conley_cutoff_km=200.0,
1944-
conley_lag_cutoff=0, # spatial-within-period only
1949+
conley_lag_cutoff=2, # within-unit Bartlett up to 2 panel periods
19451950
).fit(data, outcome="y", treatment="treated", time="period",
19461951
post_periods=[2, 3], unit="unit_id")
19471952
```
19481953

19491954
**Note on `conley_lag_cutoff` semantics:** the lag is counted in **panel
19501955
periods** (number of distinct sorted values in the `time` column), NOT in
19511956
raw-label differences. Internally, time labels are normalized to dense codes
1952-
`0..T-1` via `np.unique(return_inverse=True)`. For example, `time` values
1953-
`(2020, 2021, 2022)` and `(202012, 202101, 202102)` both produce the same
1954-
codes `(0, 1, 2)` and the same lag matrix. **Deviation from R `conleyreg`:**
1955-
R uses raw `time` values directly in the lag computation, which silently
1956-
mishandles non-dense encodings (`time = 202012, 202101` and `lag_cutoff=1`
1957-
under R produces 0 weight because the raw difference is 89). diff-diff is
1958-
the more robust default; for bit-exact R parity, pass `time` as a dense
1959-
integer index.
1957+
`0..T-1` via `np.unique(return_inverse=True)`. For example, MultiPeriodDiD
1958+
with `time` values `(2020, 2021, 2022)` and `(202012, 202101, 202102)` both
1959+
produce the same codes `(0, 1, 2)` and the same lag matrix. **TWFE caveat:**
1960+
the time-label normalization runs inside `_compute_conley_vcov`, but TWFE's
1961+
own design step `_treatment_post = treated * time` requires numeric `time`;
1962+
non-numeric labels (datetime64, pd.Period, strings) are TWFE-incompatible
1963+
end-to-end. Use MultiPeriodDiD if you need datetime/Period/string time
1964+
labels. **Deviation from R `conleyreg`:** R uses raw `time` values directly
1965+
in the lag computation, which silently mishandles non-dense encodings.
1966+
diff-diff is the more robust default; for bit-exact R parity, pass `time`
1967+
as a dense integer index.
19601968

19611969
**Variance estimator — cross-sectional:**
19621970

diff_diff/results.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ def _format_vcov_label(
5252
cluster_name: Optional[str],
5353
n_clusters: Optional[int],
5454
n_obs: Optional[int],
55+
conley_lag_cutoff: Optional[int] = None,
5556
) -> Optional[str]:
5657
"""Compose a human-readable variance-family label for summary output.
5758
@@ -77,6 +78,8 @@ def _format_vcov_label(
7778
# Cross-sectional Conley on direct LinearRegression / compute_robust_vcov,
7879
# or panel block-decomposed Conley (within-period spatial + within-unit
7980
# Bartlett serial) on MultiPeriodDiD / TwoWayFixedEffects.
81+
if conley_lag_cutoff is not None:
82+
return f"Conley spatial HAC (1999), lag_cutoff={conley_lag_cutoff}"
8083
return "Conley spatial HAC (1999)"
8184
return None
8285

@@ -130,15 +133,17 @@ class DiDResults:
130133
bootstrap_distribution: Optional[np.ndarray] = field(default=None, repr=False)
131134
# Survey design metadata (SurveyMetadata instance from diff_diff.survey)
132135
survey_metadata: Optional[Any] = field(default=None)
133-
# Variance-covariance family: "classical" | "hc1" | "hc2" | "hc2_bm".
134-
# Plus cluster_name when cluster-robust. Used by summary() to label the
135-
# SE family in the output. vcov_type='conley' is rejected at fit-time
136-
# for all panel estimators (DifferenceInDifferences/MultiPeriodDiD/TWFE)
137-
# in Phase 1; the supported Conley path is direct LinearRegression /
138-
# compute_robust_vcov on a cross-sectional design, which uses its own
139-
# result class.
136+
# Variance-covariance family: "classical" | "hc1" | "hc2" | "hc2_bm" |
137+
# "conley". Plus cluster_name when cluster-robust. Used by summary() to
138+
# label the SE family in the output. For vcov_type='conley' on
139+
# MultiPeriodDiD / TwoWayFixedEffects, `conley_lag_cutoff` carries the
140+
# within-unit Bartlett max lag (matches the constructor arg). On
141+
# DifferenceInDifferences, vcov_type='conley' is rejected at fit-time
142+
# (DiD.fit() has no unit column declaration); see MultiPeriodDiD /
143+
# TwoWayFixedEffects for the panel block-decomposed path.
140144
vcov_type: Optional[str] = field(default=None)
141145
cluster_name: Optional[str] = field(default=None)
146+
conley_lag_cutoff: Optional[int] = field(default=None)
142147

143148
def __repr__(self) -> str:
144149
"""Concise string representation."""
@@ -220,6 +225,7 @@ def summary(self, alpha: Optional[float] = None) -> str:
220225
cluster_name=self.cluster_name,
221226
n_clusters=self.n_clusters,
222227
n_obs=self.n_obs,
228+
conley_lag_cutoff=self.conley_lag_cutoff,
223229
)
224230
if label is not None:
225231
lines.append(f"{'Variance:':<25} {label:>40}")
@@ -453,11 +459,12 @@ class MultiPeriodDiDResults:
453459
n_bootstrap: Optional[int] = field(default=None)
454460
n_clusters: Optional[int] = field(default=None)
455461
# Variance-covariance family and cluster column for summary() labeling.
456-
# vcov_type='conley' is rejected at fit-time for MultiPeriodDiD (Phase 1
457-
# supports cross-sectional Conley only via direct compute_robust_vcov);
458-
# see _format_vcov_label.
462+
# vcov_type='conley' on MultiPeriodDiD uses the panel block-decomposed
463+
# form; `conley_lag_cutoff` carries the within-unit Bartlett max lag
464+
# (matches the constructor arg). See _format_vcov_label.
459465
vcov_type: Optional[str] = field(default=None)
460466
cluster_name: Optional[str] = field(default=None)
467+
conley_lag_cutoff: Optional[int] = field(default=None)
461468

462469
# --- Inference-field aliases (balance/external-adapter compatibility) ---
463470
@property
@@ -560,6 +567,7 @@ def summary(self, alpha: Optional[float] = None) -> str:
560567
cluster_name=self.cluster_name,
561568
n_clusters=self.n_clusters,
562569
n_obs=self.n_obs,
570+
conley_lag_cutoff=self.conley_lag_cutoff,
563571
)
564572
if label is not None:
565573
lines.append(f"{'Variance:':<25} {label:>50}")

diff_diff/twfe.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,7 @@ def _refit_twfe(w_r):
593593
# remapped hc1 under the legacy alias path, not self.vcov_type.
594594
vcov_type=_fit_vcov_type,
595595
cluster_name=_twfe_cluster_label,
596+
conley_lag_cutoff=(self.conley_lag_cutoff if _fit_vcov_type == "conley" else None),
596597
)
597598

598599
self.is_fitted_ = True

tests/test_conley_vcov.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1403,7 +1403,10 @@ def test_twfe_conley_binary_post_label_normalization(self, panel):
14031403

14041404
def test_twfe_conley_summary_emits_conley_label(self, panel):
14051405
"""Panel result summary must label the variance family as Conley
1406-
spatial HAC when vcov_type='conley'. Closes Codex P3."""
1406+
spatial HAC and surface `lag_cutoff` so downstream consumers can tell
1407+
which Conley variant produced the SEs. Closes Codex P3 and the
1408+
re-review P1 (Maintainability).
1409+
"""
14071410
from diff_diff import TwoWayFixedEffects
14081411

14091412
res = TwoWayFixedEffects(
@@ -1414,6 +1417,34 @@ def test_twfe_conley_summary_emits_conley_label(self, panel):
14141417
).fit(panel, outcome="y", treatment="treated", time="time", unit="unit")
14151418
summary = res.summary()
14161419
assert "Conley spatial HAC" in summary
1420+
assert "lag_cutoff=1" in summary
1421+
# The result dataclass also carries the lag for programmatic access.
1422+
assert res.conley_lag_cutoff == 1
1423+
1424+
def test_twfe_conley_non_numeric_time_fails(self, panel):
1425+
"""TWFE's `_treatment_post = treated * time` design step requires
1426+
numeric `time`. Non-numeric labels (datetime64, pd.Period, strings)
1427+
are TWFE-incompatible end-to-end and surface as a clean error before
1428+
the Conley path runs. Use MultiPeriodDiD if you need non-numeric
1429+
time labels.
1430+
"""
1431+
from diff_diff import TwoWayFixedEffects
1432+
1433+
df_str = panel.copy()
1434+
df_str["time_str"] = df_str["time"].map({0: "pre", 1: "post"})
1435+
with pytest.raises((TypeError, ValueError)):
1436+
TwoWayFixedEffects(
1437+
vcov_type="conley",
1438+
conley_coords=("lat", "lon"),
1439+
conley_cutoff_km=2000.0,
1440+
conley_lag_cutoff=1,
1441+
).fit(
1442+
df_str,
1443+
outcome="y",
1444+
treatment="treated",
1445+
time="time_str",
1446+
unit="unit",
1447+
)
14171448

14181449
def test_twfe_conley_within_vs_dummy_expansion_equivalence(self, panel):
14191450
"""FWL composability: TWFE (within-transform) + Conley should produce

0 commit comments

Comments
 (0)