Skip to content

Commit 053d2c2

Browse files
igerberclaude
andcommitted
Address PR #401 R4 review (1 P3 maintainability)
R4 verdict was "Looks good". One P3 housekeeping item addressed: P3 (maintainability): the `summary()` empty-state renderer for `path_effects == {}` rendered the same generic "No observed paths have a complete [F_g-1, F_g-1+L_max] window" text under both selectors. Under `paths_of_interest`, that text is misleading because the panel may have observed paths but none match the user's list. Branch on `_estimator_ref.paths_of_interest`: when set, the empty-state block now reads "Every path in paths_of_interest was unobserved or had a window outside L_max+1" with a pointer to the per-path 'zero observed groups' UserWarnings; under `by_path=k`, the original wording is preserved. Adds regression `test_paths_of_interest_all_unobserved_summary_distinct_text` (next to the existing `_emits_distinct_warning` test) asserting the selector-specific summary text fires and the by_path-only wording does not. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a1f6a55 commit 053d2c2

2 files changed

Lines changed: 42 additions & 3 deletions

File tree

diff_diff/chaisemartin_dhaultfoeuille_results.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,14 +1267,26 @@ def _render_path_effects_section(
12671267
if self.path_effects is None:
12681268
return
12691269
if not self.path_effects:
1270+
# Distinguish the two empty causes for paths_of_interest
1271+
# users (every requested path unobserved) from by_path=k
1272+
# users (no panel path has a complete window).
1273+
poi = getattr(self._estimator_ref, "paths_of_interest", None)
1274+
if poi is not None:
1275+
detail_lines = [
1276+
" Every path in paths_of_interest was unobserved or had a window outside L_max+1.",
1277+
" (See per-path 'zero observed groups' UserWarnings emitted at fit().)",
1278+
]
1279+
else:
1280+
detail_lines = [
1281+
" No observed paths have a complete [F_g-1, F_g-1+L_max] window.",
1282+
" (See UserWarning emitted at fit(); by_path was a no-op on this panel.)",
1283+
]
12701284
lines.extend(
12711285
[
12721286
thin,
12731287
"Treatment-Path Disaggregation".center(width),
12741288
thin,
1275-
" No observed paths have a complete [F_g-1, F_g-1+L_max] window.",
1276-
" (See UserWarning emitted at fit(); by_path / "
1277-
"paths_of_interest was a no-op on this panel.)",
1289+
*detail_lines,
12781290
thin,
12791291
"",
12801292
]

tests/test_chaisemartin_dhaultfoeuille.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8732,6 +8732,33 @@ def test_paths_of_interest_frequency_rank_is_true_frequency(self):
87328732
assert res.path_effects[(0, 1, 1, 1)]["frequency_rank"] == 1
87338733
assert res.path_effects[(0, 1, 0, 0)]["frequency_rank"] == 2
87348734

8735+
def test_paths_of_interest_all_unobserved_summary_distinct_text(self):
8736+
"""When every path in `paths_of_interest` is unobserved, the
8737+
empty-state `summary()` block must render the
8738+
paths_of_interest-specific text rather than the generic
8739+
"no observed paths have a complete window" text (regression
8740+
for R4 P3 maintainability finding)."""
8741+
df = _by_path_three_path_data()
8742+
est = ChaisemartinDHaultfoeuille(
8743+
drop_larger_lower=False,
8744+
paths_of_interest=[(1, 1, 1, 1), (1, 0, 1, 0)], # both unobserved
8745+
twfe_diagnostic=False,
8746+
seed=42,
8747+
)
8748+
with warnings.catch_warnings():
8749+
warnings.simplefilter("ignore", UserWarning)
8750+
res = est.fit(
8751+
df, outcome="outcome", group="group", time="period",
8752+
treatment="treatment", L_max=3,
8753+
)
8754+
text = res.summary()
8755+
assert "Every path in paths_of_interest was unobserved" in text, (
8756+
f"summary() did not render the paths_of_interest-specific "
8757+
f"empty-state text. Got:\n{text}"
8758+
)
8759+
# And the generic by_path-only wording must NOT appear in this case.
8760+
assert "No observed paths have a complete" not in text
8761+
87358762
def test_paths_of_interest_all_unobserved_emits_distinct_warning(self):
87368763
"""When every path in `paths_of_interest` is unobserved,
87378764
the empty-state warning should mention `paths_of_interest`

0 commit comments

Comments
 (0)