Skip to content

Commit a1f6a55

Browse files
igerberclaude
andcommitted
Address PR #401 R2 review (1 P3 maintainability)
R2 verdict was "Looks good". One residual P3 housekeeping class addressed (selector-aware messaging in additional locations): 1. Multi-baseline `UserWarning`s under `controls` and `trends_linear` now read "by_path / paths_of_interest + ..." instead of "by_path + ..." since both fire under either selector. 2. F_g=3 boundary `UserWarning` under `trends_linear` now reads "by_path / paths_of_interest + trends_linear: ..." for the same reason. 3. `summary()` non-empty header dropped the "(by_path)" suffix — matches the empty-state branch fixed in the prior round. 4. `to_dataframe(level="by_path")` docstring now documents that the surface is populated under either `by_path=k` or `paths_of_interest=[(...), ...]`. Updates 5 test assertions that matched the old "by_path + ..." warning substring; the new "+ trends_linear" / "+ controls" substring matches both selector forms. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7d29fc2 commit a1f6a55

3 files changed

Lines changed: 23 additions & 20 deletions

File tree

diff_diff/chaisemartin_dhaultfoeuille.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,12 +1722,13 @@ def fit(
17221722
_switcher_baselines = baselines[_switcher_mask]
17231723
if np.unique(_switcher_baselines).size > 1:
17241724
warnings.warn(
1725-
"by_path + controls: switcher baselines D_{g,1} "
1726-
"take multiple values in this panel. Python "
1727-
"residualizes once on the full panel before path "
1728-
"enumeration; R `did_multiplegt_dyn(..., by_path, "
1729-
"controls)` re-runs residualization per path on "
1730-
"the path-restricted subsample, so per-path point "
1725+
"by_path / paths_of_interest + controls: "
1726+
"switcher baselines D_{g,1} take multiple values "
1727+
"in this panel. Python residualizes once on the "
1728+
"full panel before path enumeration; R "
1729+
"`did_multiplegt_dyn(..., by_path, controls)` "
1730+
"re-runs residualization per path on the "
1731+
"path-restricted subsample, so per-path point "
17311732
"estimates can diverge between Python and R on "
17321733
"this panel. See `docs/methodology/REGISTRY.md` "
17331734
"(`Note (Phase 3 by_path ...)` -> Per-path "
@@ -1826,10 +1827,10 @@ def fit(
18261827
_switcher_baselines_tl = baselines[_switcher_mask_tl]
18271828
if np.unique(_switcher_baselines_tl).size > 1:
18281829
warnings.warn(
1829-
"by_path + trends_linear: switcher baselines "
1830-
"D_{g,1} take multiple values in this panel. "
1831-
"Python first-differences once on the full "
1832-
"panel before path enumeration; R "
1830+
"by_path / paths_of_interest + trends_linear: "
1831+
"switcher baselines D_{g,1} take multiple values "
1832+
"in this panel. Python first-differences once on "
1833+
"the full panel before path enumeration; R "
18331834
"`did_multiplegt_dyn(..., by_path, trends_lin)` "
18341835
"re-runs the full pipeline (including "
18351836
"first-differencing) on each path's restricted "
@@ -1861,8 +1862,9 @@ def fit(
18611862
_f_g_three_count = int((first_switch_idx_arr == 2).sum())
18621863
if _f_g_three_count > 0:
18631864
warnings.warn(
1864-
f"by_path + trends_linear: {_f_g_three_count} "
1865-
f"switching group(s) have F_g=3 (exactly 2 "
1865+
f"by_path / paths_of_interest + trends_linear: "
1866+
f"{_f_g_three_count} switching group(s) have "
1867+
f"F_g=3 (exactly 2 "
18661868
f"pre-switch periods). After first-differencing "
18671869
f"and the time==1 filter, these groups have "
18681870
f"only 1 valid pre-window Z value, which "

diff_diff/chaisemartin_dhaultfoeuille_results.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,7 +1283,7 @@ def _render_path_effects_section(
12831283
lines.extend(
12841284
[
12851285
thin,
1286-
"Treatment-Path Disaggregation (by_path)".center(width),
1286+
"Treatment-Path Disaggregation".center(width),
12871287
thin,
12881288
]
12891289
)
@@ -1442,8 +1442,9 @@ def to_dataframe(self, level: str = "overall") -> pd.DataFrame:
14421442
Available when ``trends_linear=True``.
14431443
- ``"design2"``: Design-2 switch-in/switch-out descriptive
14441444
summary. Available when ``design2=True``.
1445-
- ``"by_path"``: one row per (path, horizon) when
1446-
``by_path=k`` was passed to the estimator. Columns:
1445+
- ``"by_path"``: one row per (path, horizon) when either
1446+
``by_path=k`` or ``paths_of_interest=[(...), ...]`` was
1447+
passed to the estimator. Columns:
14471448
``path``, ``frequency_rank``, ``n_groups``, ``horizon``,
14481449
``effect``, ``se``, ``t_stat``, ``p_value``,
14491450
``conf_int_lower``, ``conf_int_upper``, ``n_obs``,

tests/test_chaisemartin_dhaultfoeuille.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6638,7 +6638,7 @@ def _add(group, treatment_path):
66386638
str(w.message)
66396639
for w in caught
66406640
if issubclass(w.category, UserWarning)
6641-
and "by_path + controls" in str(w.message)
6641+
and "+ controls" in str(w.message)
66426642
and "multi-baseline" not in str(w.message).lower()
66436643
or (
66446644
issubclass(w.category, UserWarning)
@@ -7579,7 +7579,7 @@ def _add(group, treatment_path):
75797579
str(w.message)
75807580
for w in caught
75817581
if issubclass(w.category, UserWarning)
7582-
and "by_path + trends_linear" in str(w.message)
7582+
and "+ trends_linear" in str(w.message)
75837583
and "switcher baselines" in str(w.message)
75847584
]
75857585
assert deviation_msgs, (
@@ -7609,7 +7609,7 @@ def test_single_baseline_panel_does_not_emit_r_deviation_warning(self):
76097609
str(w.message)
76107610
for w in caught
76117611
if issubclass(w.category, UserWarning)
7612-
and "by_path + trends_linear" in str(w.message)
7612+
and "+ trends_linear" in str(w.message)
76137613
and "switcher baselines" in str(w.message)
76147614
]
76157615
assert not deviation_msgs, (
@@ -7667,7 +7667,7 @@ def _add(group, treatment_path):
76677667
str(w.message)
76687668
for w in caught
76697669
if issubclass(w.category, UserWarning)
7670-
and "by_path + trends_linear" in str(w.message)
7670+
and "+ trends_linear" in str(w.message)
76717671
and "F_g=3" in str(w.message)
76727672
]
76737673
assert boundary_msgs, (
@@ -7718,7 +7718,7 @@ def test_single_baseline_heterogeneous_F_g_does_not_warn(self):
77187718
str(w.message)
77197719
for w in caught
77207720
if issubclass(w.category, UserWarning)
7721-
and "by_path + trends_linear" in str(w.message)
7721+
and "+ trends_linear" in str(w.message)
77227722
and "switcher baselines" in str(w.message)
77237723
]
77247724
assert not deviation_msgs, (

0 commit comments

Comments
 (0)