Skip to content

Commit 8a40af6

Browse files
igerberclaude
andcommitted
Fix __all__ duplicates, add hover-label regression test, defer styling kwargs to TODO
- Remove duplicate plot_bacon/plot_power_curve/plot_pretrends_power from __all__ (were listed in both category sections) - Add regression test asserting plotly event study traces preserve period labels in customdata/hovertemplate - Add __all__ uniqueness assertion to export test - Track plotly styling kwargs parity as deferred P2 in TODO.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8689b97 commit 8a40af6

3 files changed

Lines changed: 28 additions & 3 deletions

File tree

TODO.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ Deferred items from PR reviews that were not addressed before merge.
6969
| R comparison tests spawn separate `Rscript` per test (slow CI) | `tests/test_methodology_twfe.py:294` | #139 | Low |
7070
| CS R helpers hard-code `xformla = ~ 1`; no covariate-adjusted R benchmark for IRLS path | `tests/test_methodology_callaway.py` | #202 | Low |
7171
| ~376 `duplicate object description` Sphinx warnings — caused by autodoc `:members:` on dataclass attributes within manual API pages (not from autosummary stubs); fix requires restructuring `docs/api/*.rst` pages to avoid documenting the same attribute via both `:members:` and inline `autosummary` tables | `docs/api/*.rst` || Low |
72+
| Plotly renderers silently ignore styling kwargs (marker, markersize, linewidth, capsize, ci_linewidth) that the matplotlib backend honors; thread them through or reject when `backend="plotly"` | `visualization/_event_study.py`, `_diagnostic.py`, `_power.py` | #222 | Medium |
7273

7374
---
7475

diff_diff/__init__.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,6 @@
224224
"BaconDecompositionResults",
225225
"Comparison2x2",
226226
"bacon_decompose",
227-
"plot_bacon",
228227
# Results
229228
"DiDResults",
230229
"MultiPeriodDiDResults",
@@ -319,14 +318,12 @@
319318
"simulate_mde",
320319
"simulate_power",
321320
"simulate_sample_size",
322-
"plot_power_curve",
323321
# Pre-trends power analysis
324322
"PreTrendsPower",
325323
"PreTrendsPowerResults",
326324
"PreTrendsPowerCurve",
327325
"compute_pretrends_power",
328326
"compute_mdv",
329-
"plot_pretrends_power",
330327
# Survey support
331328
"SurveyDesign",
332329
"SurveyMetadata",

tests/test_visualization_plotly.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,3 +300,30 @@ def test_all_plots_in_all(self):
300300
"plot_group_time_heatmap",
301301
]:
302302
assert name in diff_diff.__all__, f"{name} missing from diff_diff.__all__"
303+
304+
def test_no_duplicates_in_all(self):
305+
import diff_diff
306+
307+
seen = set()
308+
for name in diff_diff.__all__:
309+
assert name not in seen, f"Duplicate in __all__: {name}"
310+
seen.add(name)
311+
312+
313+
class TestPlotlyEventStudyHover:
314+
"""Regression: plotly event study must preserve period labels in hover."""
315+
316+
def test_string_periods_in_customdata(self):
317+
from diff_diff import plot_event_study
318+
319+
effects = {"pre": 0.0, "post": 0.5}
320+
se = {"pre": 0.1, "post": 0.15}
321+
fig = plot_event_study(
322+
effects=effects, se=se, backend="plotly", show=False
323+
)
324+
# At least one point trace should have customdata with original labels
325+
point_traces = [t for t in fig.data if t.mode == "markers"]
326+
assert len(point_traces) > 0
327+
for trace in point_traces:
328+
assert trace.customdata is not None, "Missing customdata on point trace"
329+
assert trace.hovertemplate is not None, "Missing hovertemplate"

0 commit comments

Comments
 (0)