Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major code refactor to unify quasi experiment classes #381

Merged
merged 72 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 66 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
df1989f
Very initial work on the big refactor
drbenvincent Jun 28, 2024
d36e289
update uml diagram
drbenvincent Jun 28, 2024
0945fc4
change from dict to dataclass
drbenvincent Jun 28, 2024
d0d3bc3
Friday evening progress
drbenvincent Jun 28, 2024
f87577b
bayesian anova sorted + made plot methods static methods
drbenvincent Jul 1, 2024
e82325d
regression discontinuity sorted
drbenvincent Jul 1, 2024
4ccea2c
regression discontinuity sorted + add summary methods
drbenvincent Jul 1, 2024
119f749
rename classes
drbenvincent Jul 1, 2024
841cca4
add inverse propensity weighting
drbenvincent Jul 1, 2024
e5a07d9
add docstrings back in
drbenvincent Jul 1, 2024
7b1e600
make doctests pass
drbenvincent Jul 1, 2024
48cf9b5
make lots of tests pass
drbenvincent Jul 1, 2024
5c2b103
make test pass - Line2D
drbenvincent Jul 1, 2024
b23b373
fix import
drbenvincent Jul 1, 2024
0435456
add docstrings to make interrogate pre-commit check pass
drbenvincent Jul 1, 2024
ac2389d
Update expt_prepostnegd.py
drbenvincent Jul 1, 2024
c6ae453
fix _causal_impact_summary_stat
drbenvincent Jul 1, 2024
ff5122f
tidy up plotting
drbenvincent Jul 1, 2024
291dc47
fix score
drbenvincent Jul 1, 2024
4d10175
add convert_to_string + fix typo
drbenvincent Jul 1, 2024
4442d5b
ensure fig, ax returned from plot method
drbenvincent Jul 1, 2024
3757199
add causal impact arrow back in to DID plot
drbenvincent Jul 1, 2024
6c4e43c
zero failing tests
drbenvincent Jul 2, 2024
ae7c405
add test coverage for plot method to integration tests
drbenvincent Jul 2, 2024
a85ccfb
fix errors in tests + make some tests pass
drbenvincent Jul 2, 2024
00c1290
make a test pass
drbenvincent Jul 2, 2024
5b5ccd2
fix docstring + add type hint
drbenvincent Jul 2, 2024
c3df3eb
make another plot test pass
drbenvincent Jul 2, 2024
2fb344a
fix plotting issue with some scikit-learn models
drbenvincent Jul 2, 2024
fd74658
fix problem with GaussianProcessRegressor. Back to ZERO failing tests
drbenvincent Jul 2, 2024
41bf080
re-run most notebooks
drbenvincent Jul 2, 2024
c77de98
fix plotting of control units for synthetic control + add test coverage
drbenvincent Jul 2, 2024
ea2b859
update UML
drbenvincent Jul 2, 2024
b0b4539
tweaks
drbenvincent Jul 2, 2024
b0dc8c6
remove commented code
drbenvincent Jul 2, 2024
0af9bfb
pre-commit autoupdate
drbenvincent Jul 2, 2024
28f3b07
improve manual type checking sections in the experiment modules
drbenvincent Jul 2, 2024
e84c199
add/improve module level docstrings
drbenvincent Jul 3, 2024
b0eabff
add missing docstrings back in
drbenvincent Jul 3, 2024
1b26499
fix failing doctest
drbenvincent Jul 3, 2024
133ee3b
Merge branch 'main' into refactor
juanitorduz Jul 10, 2024
5c07e4d
change data validation mixins to class methods
drbenvincent Jul 11, 2024
224ec84
remove commented imports
drbenvincent Jul 11, 2024
100784f
_input_validation -> input_validation
drbenvincent Jul 11, 2024
d6e058c
change all asserts outside of tests into checks which raise exceptions
drbenvincent Jul 11, 2024
dab5824
experiment submodule
drbenvincent Jul 11, 2024
4b8141d
remove setting attributes to None in ExperimentDesign base class
drbenvincent Jul 11, 2024
47d4479
plot_ATE -> plot_ate
drbenvincent Jul 11, 2024
70e58f8
change experiment.py::ExperimentalDesign to base.py::BaseExperiment
drbenvincent Jul 11, 2024
c080fa9
remove PlotComponent abstract base class
drbenvincent Jul 11, 2024
f915f77
add return type hints for plot methods + fix up test assertions
drbenvincent Jul 11, 2024
6822c61
Handle deprecation like a grown up
drbenvincent Jul 11, 2024
1cdf7c2
convert the deprecation wrapper classes into functions
drbenvincent Jul 11, 2024
fbc4c94
move plotting into experiment classes, removing PlotComponent entirely
drbenvincent Jul 12, 2024
fa640b8
create new helper function, create_causalpy_compatible_class
drbenvincent Jul 13, 2024
4edc6d5
fix typo
drbenvincent Aug 6, 2024
38b0e68
Update causalpy/experiments/diff_in_diff.py
drbenvincent Aug 6, 2024
6a9214b
remove **kwargs being passed to BaseExperiment.__init__
drbenvincent Aug 6, 2024
0cf4f46
remove unnecessary kwargs in did experiments
drbenvincent Aug 6, 2024
60cfb2a
update uml
drbenvincent Aug 6, 2024
121fe46
remove old API pages
drbenvincent Aug 6, 2024
cc62438
better model/experiment compatability
drbenvincent Aug 7, 2024
644cf6b
move IPW integration test to better test file
drbenvincent Aug 7, 2024
dede64a
add warning to NotImplementedError
drbenvincent Aug 7, 2024
02dacb2
convert scikitlearn models behind the scenes
drbenvincent Aug 9, 2024
3f9763a
fix import in one of the tests
drbenvincent Aug 9, 2024
01ce582
improve deprecation warnings
drbenvincent Aug 9, 2024
6f6fade
fix typo
drbenvincent Aug 20, 2024
66f22aa
update deprecation warnings for the scikit-learn experiment classes
drbenvincent Aug 20, 2024
77b0b2b
Merge branch 'main' into refactor
drbenvincent Aug 20, 2024
9bc3d25
depricated -> deprecated
drbenvincent Aug 22, 2024
e0b0847
remove redundant sample_kwargs definition in test
drbenvincent Aug 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,12 @@ We recommend that your contribution complies with the following guidelines befor
make doctest
```

- Doctest can also be run directly via pytest, which can be helpful to run only specific tests during development. The following commands run all doctests, only doctests in the pymc_models module, and only the doctests for the `ModelBuilder` class in pymc_models:
- Doctest can also be run directly via pytest, which can be helpful to run only specific tests during development. The following commands run all doctests, only doctests in the pymc_models module, and only the doctests for the `PyMCModel` class in pymc_models:

```bash
pytest --doctest-modules causalpy/
pytest --doctest-modules causalpy/pymc_models.py
pytest --doctest-modules causalpy/pmyc_models.py::causalpy.pymc_models.ModelBuilder
pytest --doctest-modules causalpy/pmyc_models.py::causalpy.pymc_models.PyMCModel
```

- To indicate a work in progress please mark the PR as `draft`. Drafts may be useful to (1) indicate you are working on something to avoid duplicated work, (2) request broad review of functionality or API, or (3) seek collaborators.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ df = (
)

# Run the analysis
result = cp.pymc_experiments.RegressionDiscontinuity(
result = cp.RegressionDiscontinuity(
df,
formula="all ~ 1 + age + treated",
running_variable_name="age",
Expand Down
30 changes: 25 additions & 5 deletions causalpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,38 @@
# limitations under the License.
import arviz as az

from causalpy import pymc_experiments, pymc_models, skl_experiments, skl_models
import causalpy.pymc_experiments as pymc_experiments # to be depricated
import causalpy.pymc_models as pymc_models
import causalpy.skl_experiments as skl_experiments # to be depricated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import causalpy.pymc_experiments as pymc_experiments # to be depricated
import causalpy.pymc_models as pymc_models
import causalpy.skl_experiments as skl_experiments # to be depricated
import causalpy.pymc_experiments as pymc_experiments # to be deprecated
import causalpy.pymc_models as pymc_models
import causalpy.skl_experiments as skl_experiments # to be deprecated

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - did a global find/replace

import causalpy.skl_models as skl_models
from causalpy.skl_models import create_causalpy_compatible_class
from causalpy.version import __version__

from .data import load_data
from .experiments.diff_in_diff import DifferenceInDifferences
from .experiments.instrumental_variable import InstrumentalVariable
from .experiments.inverse_propensity_weighting import InversePropensityWeighting
from .experiments.prepostfit import InterruptedTimeSeries, SyntheticControl
from .experiments.prepostnegd import PrePostNEGD
from .experiments.regression_discontinuity import RegressionDiscontinuity
from .experiments.regression_kink import RegressionKink

az.style.use("arviz-darkgrid")

__all__ = [
"pymc_experiments",
"__version__",
"DifferenceInDifferences",
"create_causalpy_compatible_class",
"InstrumentalVariable",
"InterruptedTimeSeries",
"InversePropensityWeighting",
"load_data",
"PrePostNEGD",
"pymc_experiments", # to be depricated
"pymc_models",
"skl_experiments",
"RegressionDiscontinuity",
"RegressionKink",
"skl_experiments", # to be depricated
"skl_models",
"load_data",
"__version__",
"SyntheticControl",
]
174 changes: 0 additions & 174 deletions causalpy/data_validation.py

This file was deleted.

13 changes: 13 additions & 0 deletions causalpy/experiments/__init__.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you interested in the imports like from causalpy.experiments import DifferenceInDifferences or just import causalpy as cp; cp.DifferenceInDifferences(...)
Just mentioning because that is the recommendation in deprecation warning. However, it doesn't work without adding those imports here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear on this comment. See a response below #381 (comment)

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Copyright 2024 The PyMC Labs Developers
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
80 changes: 80 additions & 0 deletions causalpy/experiments/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Copyright 2024 The PyMC Labs Developers
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""
Base class for quasi experimental designs.
"""

from abc import abstractmethod

from sklearn.base import RegressorMixin

from causalpy.pymc_models import PyMCModel
from causalpy.skl_models import create_causalpy_compatible_class


class BaseExperiment:
"""Base class for quasi experimental designs."""
wd60622 marked this conversation as resolved.
Show resolved Hide resolved

supports_bayes: bool
supports_ols: bool

def __init__(self, model=None):
# Ensure we've made any provided Scikit Learn model (as identified as being type
# RegressorMixin) compatible with CausalPy by appending our custom methods.
if isinstance(model, RegressorMixin):
model = create_causalpy_compatible_class(model)

if model is not None:
self.model = model

if isinstance(self.model, PyMCModel) and not self.supports_bayes:
raise ValueError("Bayesian models not supported.")

Check warning on line 42 in causalpy/experiments/base.py

View check run for this annotation

Codecov / codecov/patch

causalpy/experiments/base.py#L42

Added line #L42 was not covered by tests

if isinstance(self.model, RegressorMixin) and not self.supports_ols:
raise ValueError("OLS models not supported.")

if self.model is None:
raise ValueError("model not set or passed.")

Check warning on line 48 in causalpy/experiments/base.py

View check run for this annotation

Codecov / codecov/patch

causalpy/experiments/base.py#L48

Added line #L48 was not covered by tests

@property
def idata(self):
"""Return the InferenceData object of the model. Only relevant for PyMC models."""
return self.model.idata

def print_coefficients(self, round_to=None):
"""Ask the model to print its coefficients."""
self.model.print_coefficients(self.labels, round_to)

def plot(self, *args, **kwargs) -> tuple:
"""Plot the model.

Internally, this function dispatches to either `bayesian_plot` or `ols_plot`
depending on the model type.
"""
if isinstance(self.model, PyMCModel):
return self.bayesian_plot(*args, **kwargs)
elif isinstance(self.model, RegressorMixin):
return self.ols_plot(*args, **kwargs)
else:
raise ValueError("Unsupported model type")

Check warning on line 70 in causalpy/experiments/base.py

View check run for this annotation

Codecov / codecov/patch

causalpy/experiments/base.py#L70

Added line #L70 was not covered by tests

@abstractmethod
def bayesian_plot(self, *args, **kwargs):
"""Abstract method for plotting the model."""
raise NotImplementedError("bayesian_plot method not yet implemented")

Check warning on line 75 in causalpy/experiments/base.py

View check run for this annotation

Codecov / codecov/patch

causalpy/experiments/base.py#L75

Added line #L75 was not covered by tests

@abstractmethod
def ols_plot(self, *args, **kwargs):
"""Abstract method for plotting the model."""
raise NotImplementedError("ols_plot method not yet implemented")

Check warning on line 80 in causalpy/experiments/base.py

View check run for this annotation

Codecov / codecov/patch

causalpy/experiments/base.py#L80

Added line #L80 was not covered by tests
Loading
Loading