pymc-labs / CausalPy

A Python package for causal inference in quasi-experimental settings
https://causalpy.readthedocs.io
Apache License 2.0
829 stars 52 forks source link

Add example analysis of multiple geo lift test analysis #338

Closed drbenvincent closed 1 week ago

drbenvincent commented 1 month ago

Create new classes?

The PR currently does not add any additional code or classes. All the new functionality is embedded within the example notebook. It is relatively simple, we are just creating an aggregate treated region or iterating through each treated geo. So at the moment this uses an approach of transparency so that users can see what is being done etc.

However, part of the point of CausalPy is to provide a simple to use API, not requiring the user to do that much manual python coding. So the question is whether we should create new classes / an API. It would be something along the lines of:

For the pooled approach:

result = multi_cell_geo_test_aggregate(
    df, 
    agg_func=median,
    experiment=cp.pymc_experiments.SyntheticControl,
    expt_kwargs={"treatment_time": treatment_time,
                 "formula": formula},
    model=cp.pymc_models.WeightedSumFitter,
    model_kwargs={sample_kwargs: {"target_accept": 0.95, "random_seed": seed}},
)

For the unpooled approach:

results = multi_cell_geo_test_unpooled(
    df,
    treated_geos=treated,
    untreated_geos=untreated,
    experiment=cp.pymc_experiments.SyntheticControl,
    expt_kwargs={"treatment_time": treatment_time,
                 "formula": formula},
    model=cp.pymc_models.WeightedSumFitter,
    model_kwargs={sample_kwargs: {"target_accept": 0.95, "random_seed": seed}},
)

So the trade-off would be having a relatively clean API, but at the expense of making the operation a little more opaque. The manual python code in the notebook (as it stands right now) is not that complex. So I think it's not overwhelmingly obvious which we should go with.

TODO's based on feedback so far

review-notebook-app[bot] commented 1 month ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.60%. Comparing base (9330a9c) to head (f89c53b).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #338 +/- ## ========================================== + Coverage 83.10% 85.60% +2.49% ========================================== Files 21 22 +1 Lines 1687 1716 +29 ========================================== + Hits 1402 1469 +67 + Misses 285 247 -38 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2024-05-08T12:57:25Z ----------------------------------------------------------------

nip: use " or ' for strings (not both) ... btw: do we have ruff for notebooks? :)


drbenvincent commented on 2024-05-08T19:57:06Z ----------------------------------------------------------------

Fixed. Created https://github.com/pymc-labs/CausalPy/issues/340 to double check ruff for notebooks.