Skip to content

Commit 710f2c1

Browse files
igerberclaude
andcommitted
Update docs and reuse fixture per PR review feedback
- Clarify reference_period docstring: explicit triggers normalization, auto-inferred only applies hollow marker styling - Update Notes section to distinguish explicit vs inferred behavior - Refactor two tests to reuse cs_results fixture instead of re-fitting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 60c04cb commit 710f2c1

2 files changed

Lines changed: 13 additions & 28 deletions

File tree

diff_diff/visualization.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,10 @@ def plot_event_study(
7373
periods : list, optional
7474
List of periods to plot. If None, uses all periods from results.
7575
reference_period : any, optional
76-
The reference period (normalized to effect=0). Will be shown as a
77-
hollow marker. If None, tries to infer from results.
76+
The reference period to highlight. When explicitly provided, effects
77+
are normalized (ref effect subtracted) and ref SE is set to NaN.
78+
When None and auto-inferred from results, only hollow marker styling
79+
is applied (no normalization). If None, tries to infer from results.
7880
pre_periods : list, optional
7981
List of pre-treatment periods. Used for shading.
8082
post_periods : list, optional
@@ -151,8 +153,9 @@ def plot_event_study(
151153
trends holds. Large pre-treatment effects suggest the assumption may
152154
be violated.
153155
154-
2. **Reference period**: Usually the last pre-treatment period (t=-1),
155-
normalized to zero. This is the omitted category.
156+
2. **Reference period**: Usually the last pre-treatment period (t=-1).
157+
When explicitly specified via ``reference_period``, effects are normalized
158+
to zero at this period. When auto-inferred, shown with hollow marker only.
156159
157160
3. **Post-treatment periods**: The treatment effects of interest. These
158161
show how the outcome evolved after treatment.

tests/test_visualization.py

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ def test_plot_event_study_normalization_with_nan_reference(self):
456456

457457
plt.close()
458458

459-
def test_plot_cs_results_no_auto_normalization(self):
459+
def test_plot_cs_results_no_auto_normalization(self, cs_results):
460460
"""Test that auto-inferred reference period does NOT normalize effects.
461461
462462
When CallawaySantAnna results auto-infer reference_period=-1 (or from n_groups=0),
@@ -466,17 +466,8 @@ def test_plot_cs_results_no_auto_normalization(self):
466466
pytest.importorskip("matplotlib")
467467
import matplotlib.pyplot as plt
468468

469-
# Generate staggered data and fit CallawaySantAnna with event study
470-
data = generate_staggered_data()
471-
cs = CallawaySantAnna()
472-
results = cs.fit(
473-
data,
474-
outcome='outcome',
475-
unit='unit',
476-
time='time',
477-
first_treat='first_treat',
478-
aggregate='event_study'
479-
)
469+
# Use fixture instead of re-fitting
470+
results = cs_results
480471

481472
# Get original effects from results (before any normalization)
482473
original_effects = {
@@ -513,7 +504,7 @@ def test_plot_cs_results_no_auto_normalization(self):
513504

514505
plt.close()
515506

516-
def test_plot_cs_results_explicit_reference_normalizes(self):
507+
def test_plot_cs_results_explicit_reference_normalizes(self, cs_results):
517508
"""Test that explicit reference_period normalizes CallawaySantAnna results.
518509
519510
When user explicitly passes reference_period=X to plot_event_study,
@@ -522,17 +513,8 @@ def test_plot_cs_results_explicit_reference_normalizes(self):
522513
pytest.importorskip("matplotlib")
523514
import matplotlib.pyplot as plt
524515

525-
# Generate staggered data and fit CallawaySantAnna with event study
526-
data = generate_staggered_data()
527-
cs = CallawaySantAnna()
528-
results = cs.fit(
529-
data,
530-
outcome='outcome',
531-
unit='unit',
532-
time='time',
533-
first_treat='first_treat',
534-
aggregate='event_study'
535-
)
516+
# Use fixture instead of re-fitting
517+
results = cs_results
536518

537519
# Get original effects from results
538520
original_effects = {

0 commit comments

Comments
 (0)