[Repo Assist] fix(estimators): default target_units to "ate" in propensity score estimators - #1737
Draft
github-actions[bot] wants to merge 1 commit into
Conversation
…timators
All three propensity score estimators (Weighting, Matching, Stratification)
had target_units=None as the default in their estimate_effect() method.
When called without specifying target_units, the None value fell through
all the if/elif branches and raised:
ValueError("Target units string value not supported")
The module-level estimate_effect() and CausalModel.estimate_effect()
both default to target_units="ate", so the natural expectation when
calling an estimator directly is the same default.
Fix: change target_units=None to target_units: str = "ate" in all three
propensity score estimators.
Add regression tests in test_propensity_score_weighting_estimator.py
to verify that calling estimate_effect() without target_units succeeds
for all three estimators.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
78 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This is an automated PR from Repo Assist, an AI assistant for this repository.
Problem
All three propensity score estimators —
PropensityScoreWeightingEstimator,PropensityScoreMatchingEstimator, andPropensityScoreStratificationEstimator— hadtarget_units=Noneas the default for theirestimate_effect()method.When called without specifying
target_units, theNonevalue falls through allif/elifbranches and raises:This is a latent bug that is hidden in the standard workflow because:
CausalModel.estimate_effect()defaults totarget_units="ate"and always passes it throughestimate_effect()function also defaults totarget_units="ate"But it becomes visible when a user creates an estimator directly and calls
estimate_effect():Root Cause
The signature of
estimate_effect()in all three propensity score estimators:The
Nonedefault was inconsistent with the calling conventions of both the module-level API (default"ate") andCausalModel(default"ate").Fix
Changed the default to match the rest of the API:
Files changed:
dowhy/causal_estimators/propensity_score_weighting_estimator.pydowhy/causal_estimators/propensity_score_matching_estimator.pydowhy/causal_estimators/propensity_score_stratification_estimator.pyTests
Three regression tests added to
tests/causal_estimators/test_propensity_score_weighting_estimator.py:test_psw_estimate_effect_default_target_units_does_not_raise— PSW estimatortest_psm_estimate_effect_default_target_units_does_not_raise— PSM estimatortest_pss_estimate_effect_default_target_units_does_not_raise— PSS estimatorEach test creates a minimal confounded binary-treatment dataset, fits the estimator, and calls
estimate_effect(data)without specifyingtarget_units— verifying no exception is raised and the result is finite.Test Status
Format: ✅
black --checkpasses (no changes needed to source files).Lint: ✅ No new hard flake8 errors (E9/F63/F7/F82). Pre-existing soft warnings (E501 line length) in the estimator files are unchanged.
Tests: ⏳ CI blocked pending maintainer approval of bot PR — test suite cannot be executed in this environment (missing Poetry/venv). The fix logic has been verified against the code path.