Clean up experiments/: typos, annotations, and constants#796
Clean up experiments/: typos, annotations, and constants#796louismagowan wants to merge 10 commits intopymc-labs:mainfrom
Conversation
- diff_in_diff.py: "treament" -> "treatment" in plot label - prepostnegd.py: "trestment" -> "treatment" in comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ethods `**kwargs: dict` is incorrect — it annotates each value as a dict, not the collection. Every experiment except inverse_propensity_weighting (which already used Any) is fixed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use idiomatic `if not func(...)` instead of `if func(...) is False`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The docstring was after the first executable line; move it to the correct position immediately after the method signature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- interrupted_time_series.py: remove dead class attribute (overwritten by self.expt_type in __init__) - piecewise_its.py: move class attribute to self.expt_type in __init__ - Update test to check expt_type as instance attribute Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Create a shared constants module and replace the per-file `LEGEND_FONT_SIZE = 12` definitions in all 8 experiment files with an import from `causalpy.experiments.constants`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add `HDI_PROB: float = 0.94` to `causalpy/utils.py` and replace all hardcoded `0.94` parameter defaults and `0.03` / `1 - 0.03` quantile bounds with expressions derived from HDI_PROB. Updated files: utils.py, plot_utils.py, pymc_models.py, and 6 experiment modules (prepostnegd, regression_discontinuity, regression_kink, interrupted_time_series, piecewise_its, staggered_did, synthetic_control). Test files and docstrings are intentionally left unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #796 +/- ##
==========================================
+ Coverage 92.42% 93.79% +1.37%
==========================================
Files 52 78 +26
Lines 9230 11883 +2653
Branches 562 696 +134
==========================================
+ Hits 8531 11146 +2615
- Misses 527 545 +18
- Partials 172 192 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…nd UP038 - Move HDI_PROB after all imports in utils.py to avoid E402 - Sort `from causalpy.experiments.constants` alphabetically with other causalpy imports - Wrap long quantile lines for ruff format compliance - Fix pre-existing UP038: use `X | Y` instead of `(X, Y)` in isinstance - Restore `from typing import Any, Literal` in regression_discontinuity.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
juanitorduz
left a comment
juanitorduz
left a comment
There was a problem hiding this comment.
Thanks @louismagowan ! This looks great. I just let a minor question :)
There was a problem hiding this comment.
Thanks for the cleanup — this is thorough and well-structured. All CI green, coverage maintained. A couple of things to address:
1. Hardcoded "94%" label strings should derive from HDI_PROB
The quantile computations now correctly use HDI_PROB, but the display labels are still hardcoded as "94%". This means the constant extraction is half-done — if someone changes HDI_PROB, the math updates but the labels would be wrong.
Locations:
pymc_models.py:420—"94% HDI [...]"utils.py:111—r"$CI_{94\%}$"regression_kink.py:279—r"$CI_{94\%}$"regression_discontinuity.py:338—r"$CI_{94\%}$"prepostnegd.py:219—r"$CI_{94%}$"(also note: missing\before%, unlike the other files)
These should derive from the constant, e.g. f"{HDI_PROB*100:.0f}% HDI" and rf"$CI_{{{HDI_PROB*100:.0f}\%}}$".
2. Agree with @juanitorduz — move HDI_PROB into constants.py
Having constants split across experiments/constants.py (for LEGEND_FONT_SIZE) and utils.py (for HDI_PROB) is a bit scattered. Since HDI_PROB is used outside experiments/ too (utils.py, plot_utils.py, pymc_models.py), a top-level causalpy/constants.py might be the cleanest home for both constants. That way there's one obvious place to look.
|
Just following up on this @louismagowan. Do you think you'd be able to make the updates? Then we can merge :) |
…e CI labels from HDI_PROB - Create causalpy/constants.py as single home for shared constants - Delete causalpy/experiments/constants.py (now unused) - Update all imports across 10 files to use causalpy.constants directly - Fix 5 hardcoded "94%" label strings to derive from HDI_PROB (pymc_models, utils, regression_kink, regression_discontinuity, prepostnegd); also fix missing backslash before % in prepostnegd CI string Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@drbenvincent - how's it look now? Everything resolved? |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 participants
Summary
Purely mechanical cleanup of causalpy/experiments/ — no behavioural changes.
Closes: #795