Add causal panel problem animation to README#32
Add causal panel problem animation to README#32fedemolina wants to merge 1 commit intoTianyiPeng:mainfrom
Conversation
886e35e to
fdc952b
Compare
fdc952b to
1c18cf2
Compare
TianyiPeng
left a comment
There was a problem hiding this comment.
Great addition to the README — the causal panel problem section is well-written and the visuals make the counterfactual reconstruction problem concrete for newcomers.
What works well
- Clear mathematical framing:
O = M + τZ + εconnects the matrix formulation to the library's estimators. - Well-written prose: bridges the gap between the abstract math and the intuition behind SC, SDID, and matrix-completion approaches.
- Reproducible asset script:
make_causal_panel_assets.pyis self-contained, usesAggbackend for headless rendering, and documents how to run it. - Smooth animation: the easing transitions between stages avoid jarring cuts; the 5-stage narrative (baseline → treatment → observed → hidden counterfactual → estimate) tells a clear story.
- Good color choices: accessible, consistent palette; treated region outlined clearly.
Issues
1. GIF file size — check it won't bloat the repo
Binary files can't be reviewed from the diff. The PR says 45 frames / 30s duration at 122 DPI. At 8.8×4.9 inches that's ~1074×598px per frame. A 45-frame GIF at that resolution could easily be 5-15 MB, which will slow down README rendering on GitHub and bloat the git history permanently (since binary files don't delta-compress well). Worth checking:
ls -lh docs/assets/causal_panel_decomposition.gif- If it's >3 MB, consider reducing DPI (100 is usually fine for web), reducing
transition_stepsfrom 10 to 6, or using a smaller canvas.
2. Script dependencies not documented
make_causal_panel_assets.py requires matplotlib, numpy, and PIL (Pillow). None of these are listed as dev dependencies in pyproject.toml (matplotlib and Pillow specifically). Since the script is manual-run, it won't break package installs, but a comment at the top noting pip install matplotlib pillow (or adding them to a [tool.poetry.group.docs.dependencies] group) would help contributors.
3. Python 3.10+ syntax in the script
def build_example_panel() -> tuple[np.ndarray, ...] uses lowercase tuple (PEP 604, Python 3.10+). If the package supports Python 3.8/3.9, this will fail. The from __future__ import annotations at the top defers evaluation so it will work at runtime, but type checkers targeting older Pythons may flag it. Since this is a standalone asset script (not imported by the package), it's fine in practice — just worth noting.
4. build_example_panel() is called per animation frame
Each of the 5 calls to render_animation_frame() calls build_example_panel() independently. Since the data is deterministic (seeded rng), the results are identical. Hoisting the call to draw_animation_asset() and passing the arrays would avoid ~4 redundant SVD/convolution computations. Minor for a one-shot script, but easy to fix.
Smaller notes
- The static PNG (
causal_panel_problem.png) is also a binary — same file-size concern, though PNGs compress better than GIFs. - The README
<p align="center">+<img>pattern renders well on GitHub but not in all Markdown viewers. Standard practice, just noting. - The paper references link to arXiv and JSTOR — both stable, good.
Overall this is a nice contribution to the project's documentation. Address the file-size check and it's ready to go.
Generated by Claude Code
Summary
O = M + tau * Z + epsilon.Why
The current README is hard to read for newcomers. These visuals make the counterfactual reconstruction problem more concrete and connect the package's estimators to the matrix completion, synthetic control, and synthetic difference-in-differences intuition.
Closes #27.
Validation
.venv/bin/python docs/assets/make_causal_panel_assets.py.venv/bin/python -m py_compile docs/assets/make_causal_panel_assets.pygit diff --check