Jmafoster1/marie kondo

[jmafoster1]

Does it spark joy?

This PR gets rid of a load of relic infrastructure classes that no longer spark joy. The result is a much more streamlined testing workflow. One thing that would be nice would be to improve the structure of CausalTestCase identification and Estimator objects so that we don't need to repeat the treatment and outcome variables three times for each test.

Closes #398 #394 #392

DO NOT MERGE BEFORE #401

[github-actions]

🦙 MegaLinter status: ⚠️ WARNING

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
⚠️ PYTHON	black	31		1	1.19s
✅ PYTHON	pylint	31		0	6.1s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

[jmafoster1]

OK, so looking at the CausalTestCase class, it seems that the treatment_variable and outcome_variable properties aren't ever actually used (except in serialising to/from JSON), so we could get rid of them altogether and just store them in the associated Estimator. Programmatically this makes sense, but intuitively I'm not keen. The vibes are off - it puts too much responsibility on the "estimator". However, since the estimator also stores the adjustment set and (in the case of regression), the equational relationship, I think it might make sense to rename the Estimator family to something like CausalRelationship or something from the world of structural equational models. What are other people's thoughts?

For reference, instantiating a causal test case currently looks like this:

estimation_model = LinearRegressionEstimator(
            treatment_variable="A",
            outcome_variable="C",
            treatment_value=self.treatment_value,
            control_value=self.control_value,
            adjustment_set=self.minimal_adjustment_set,
        )
        causal_test_case = CausalTestCase(
            treatment_variable="A",
            outcome_variable="C",
            expected_causal_effect=self.expected_causal_effect,
            estimator=estimation_model,
            effect_measure="ate",
        )

In the new system, it'd look like this, which would be a lot cleaner:

causal_relationship = LinearRegressionRelationship(
            treatment_variable="A",
            outcome_variable="C",
            treatment_value=self.treatment_value,
            control_value=self.control_value,
            adjustment_set=self.minimal_adjustment_set,
        )
        causal_test_case = CausalTestCase(
            expected_causal_effect=self.expected_causal_effect,
            causal_relationship=causal_relationship,
            effect_measure="ate",
        )

[codecov]

Codecov Report

❌ Patch coverage is 94.36153% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.38%. Comparing base (8f04b99) to head (f4b077c).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
causal_testing/__main__.py	89.74%	8 Missing ⚠️
...esting/estimation/abstract_regression_estimator.py	88.67%	6 Missing ⚠️
causal_testing/testing/causal_test_case.py	93.33%	4 Missing ⚠️
causal_testing/discovery/abstract_discovery.py	91.66%	3 Missing ⚠️
causal_testing/estimation/abstract_estimator.py	76.92%	3 Missing ⚠️
causal_testing/specification/causal_dag.py	95.16%	3 Missing ⚠️
causal_testing/causal_testing_framework.py	98.09%	2 Missing ⚠️
causal_testing/testing/causal_effect.py	96.29%	2 Missing ⚠️
...ausal_testing/estimation/cubic_spline_estimator.py	92.30%	1 Missing ⚠️
...ting/estimation/instrumental_variable_estimator.py	90.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #402      +/-   ##
==========================================
- Coverage   98.15%   95.38%   -2.77%     
==========================================
  Files          31       26       -5     
  Lines        1841     1539     -302     
==========================================
- Hits         1807     1468     -339     
- Misses         34       71      +37     

Files with missing lines	Coverage Δ
causal_testing/discovery/hill_climber_discovery.py	98.24% <100.00%> (+0.03%)	⬆️
...ausal_testing/estimation/experimental_estimator.py	100.00% <100.00%> (ø)
causal_testing/estimation/ipcw_estimator.py	99.25% <100.00%> (-0.02%)	⬇️
..._testing/estimation/linear_regression_estimator.py	100.00% <100.00%> (ø)
...esting/estimation/logistic_regression_estimator.py	100.00% <100.00%> (ø)
...ing/estimation/multinomial_regression_estimator.py	100.00% <100.00%> (ø)
causal_testing/testing/causal_test_result.py	100.00% <100.00%> (+18.18%)	⬆️
causal_testing/testing/dag_adequacy.py	100.00% <100.00%> (ø)
...ausal_testing/estimation/cubic_spline_estimator.py	93.10% <92.30%> (-4.27%)	⬇️
...ting/estimation/instrumental_variable_estimator.py	96.29% <90.00%> (-3.71%)	⬇️
... and 9 more

... and 2 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b5b62a...f4b077c. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jmafoster1]

Update: Turns out removing treatment_variable and outcome_variable from CausalTestCase was really easy as nothing depended on it! I've added in treatment_variable and outcome_variable property methods for backwards compatibility and easy access to the variables, but the specification of test cases is a lot cleaner now. I think I will rename the Estimator classes to CausalRelationship to create a better intuition of what they do. This will essentially allow us to create full structural causal models!