[Repo Assist] fix(dummy_outcome_refuter): correct preprocess_data_by_treatment bugs - #1720
Draft
github-actions[bot] wants to merge 1 commit into
Conversation
Five bugs fixed in preprocess_data_by_treatment():
1. assert -> ValueError: assert can be disabled with -O; use an
explicit ValueError('single treatment variable') instead.
2. num_bins scalar bug: data.max() - data.min() returns a per-column
Series, not a scalar, breaking pd.cut(). Fix: compute the range
of the treatment column only and cast to int with a minimum of 1.
3. input mutation: data["bins"] = ... and data.drop(..., inplace=True)
mutate the caller's DataFrame, which is surprising and incompatible
with pandas 3 Copy-on-Write semantics. Fix: work on data.copy().
4. dead-code overwrite: in the continuous branch 'data = data' was a
no-op; removed.
5. unreachable categorical branch: pandas names its categorical dtype
'category', not 'categorical', so 'categorical' in 'category' is
always False. The categorical branch was dead code and also contained
a second bug (groups immediately overwritten by groupby('bins') which
doesn't exist). Fix: change guard to 'categor' in variable_type.name,
remove overwrite, add observed=True for pandas 3.x compatibility.
Also adds observed=True to the bool and categorical groupby calls to
silence the pandas FutureWarning about observed= defaulting to True.
Three new unit tests:
- test_preprocess_data_by_treatment_multiple_treatments_raises
- test_preprocess_data_by_treatment_continuous_treatment
- test_preprocess_data_by_treatment_categorical_treatment
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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 PR was created by Repo Assist, an automated AI assistant.
Related to #1665 (pandas 3.x + scikit-learn compatibility audit).
Problem
preprocess_data_by_treatment()indowhy/causal_refuters/dummy_outcome_refuter.pycontained five bugs, some of which had been present since the function's introduction.Bug 1 —
assertinstead ofValueErrorPython's
assertcan be disabled with-O(optimised mode). Production code should use an explicitValueError.Bug 2 —
num_binscomputed from a per-column Series, not a scalardata.max() - data.min()returns a Series (the range of all columns), not the scalar range of the treatment column. Passing a Series topd.cut(data[t], num_bins)works coincidentally only if the column ranges happen to be monotonically increasing; otherwisepd.cutraisesValueError: bins must increase monotonically.Fix: compute the scalar range from the treatment column only:
Bug 3 — Input DataFrame mutated
The function called
data["bins"] = pd.cut(...)anddata.drop("bins", ..., inplace=True)on the caller's DataFrame. This is surprising behaviour and is incompatible with pandas 3 Copy-on-Write semantics, which returns a read-only view for sliced DataFrames.Fix: call
data = data.copy()at the start of the function.Bug 4 — Dead no-op line
Removed.
Bug 5 — Unreachable categorical branch + immediate overwrite
pandasnames its categorical dtype"category", not"categorical". Since"categorical" in "category"isFalse, this entire branch was dead code and could never execute.The second line also immediately overwrote the first, and the
"bins"column was never created at that point.Fix: change the guard to
"categor" in variable_type.name(a substring of both"category"and"categorical"), remove the overwrite.Additional —
observed=Truemissing on groupby callsAll three
groupbycalls now includeobserved=Trueto suppress the pandas FutureWarning about the default changing.Changes
Only two files changed:
dowhy/causal_refuters/dummy_outcome_refuter.pypreprocess_data_by_treatmenttests/causal_refuters/test_dummy_outcome_refuter.pyTest Status
3 new regression tests — all pass:
test_preprocess_data_by_treatment_multiple_treatments_raisesValueError(notAssertionError)test_preprocess_data_by_treatment_continuous_treatmenttest_preprocess_data_by_treatment_categorical_treatmentpd.Categorical) now reachable and returns correct groupsblack --check,flake8 --select=E9,F63,F7,F82: no errors.