Deprecate positional treatment/y across all learners for sklearn Pipeline compat (#854) - #975
Merged
Merged
Conversation
Kick off the scikit-learn Pipeline-compatibility change for meta-learners: the positional fit() argument order will move from (X, treatment, y, ...) to (X, y, treatment, ...) in v1.0. This is the first, non-breaking step of a two-step deprecation. - Add a re-entrancy-guarded shim in BaseLearner that emits a FutureWarning when treatment/y are passed positionally to fit/fit_predict/estimate_ate, steering callers to order-independent keyword arguments. It is wired in via __init_subclass__ so every S/T/X/R/DR learner (and its subclasses) is covered with no per-method edits, and the real signatures are preserved via functools.wraps. The guard ensures internal delegation (fit_predict -> fit, subclass fit -> super().fit, estimate_ate -> fit_predict -> fit) warns at most once per top-level call. - The positional order is UNCHANGED for now, so no existing call silently breaks; keyword calls -- fit(X, y=y, treatment=treatment) -- are silent and stay correct across the v1.0 flip. - Add tests/test_fit_arg_order.py covering the warning on positional use, keyword silence, single-warning delegation, subclass super().fit, signature preservation, and positional==keyword equivalence (guarding against a silent y/treatment swap).
jeongyoonlee
marked this pull request as ready for review
July 27, 2026 01:05
jeongyoonlee
requested review from
alexander-pv,
huigangchen,
paullo0106,
ppstacy,
ras44,
t-tte,
vincewu51 and
zhenyuz0500
July 27, 2026 01:05
ras44
approved these changes
Jul 27, 2026
Step one of #854 warned only on the BaseLearner meta-learners. A deprecation window is one-shot: anything whose positional order changes at v1.0 has to warn in this release or it needs a second cycle. Widen the shim so v1.0 can flip the whole package at once. - Move the shim to causalml/inference/_arg_order.py and derive the target order from each method's own signature: X, then y, then treatment, with every other parameter keeping its relative position. Each warning now names that method's real order instead of assuming (X, treatment, y, ...). BaseDRIVLearner.fit therefore resolves to (X, y, treatment, assignment, ...) and IVRegressor.fit to (X, y, treatment, w). - Host __init_subclass__ on SerializableLearner rather than BaseLearner. That one move covers the causal/uplift trees and forests and the IV learners as well as the meta-learners. The trees matter most: they already inherit sklearn's ForestRegressor/BaseDecisionTree and override fit(X, y, ...) with fit(X, treatment, y, ...), so leaving them behind would ship v1.0 with the most sklearn-shaped classes keeping the least sklearn-shaped signature. - Decorate the six learners outside that hierarchy: the TF/JAX DragonNet, the Torch/JAX CEVAE, PolicyLearner and TMLELearner. - Shim predict too. Its (X, treatment, y, ...) is unaffected by Pipeline, but without this v1.0 would leave fit and predict holding the same two arguments in opposite orders on one class. Methods without treatment/y, such as BaseRLearner.predict, are returned unwrapped. Tests go 19 -> 43, including a completeness guard that walks the package and fails if any method taking treatment and y positionally lacks the shim, so a learner added later cannot miss the window. Refs #854. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Jul 31, 2026
The shim keyed off four method names (fit/fit_predict/estimate_ate/predict). A scan for public methods taking `treatment` before `y` found eleven more that got no warning at all: four `bootstrap` variants, `fit_bootstrap_ensemble`, `bootstrap_pool`, `prune`, `fill`, and the three `Sensitivity.get_*` helpers. A deprecation window is one-shot, so a method missing from it needs a second cycle of its own. Replace the name allowlist with the property we actually care about: shim any method whose positional order changes at v1.0. That fixes the class of bug rather than the eleven instances, and the completeness test now scans every public method instead of the same four names it was blind to. Resolves both open questions on the epic: - `UpliftTreeClassifier.fit`'s validation triple is reordered in place, so it stays consistent with the main one: (X, y, treatment, X_val, y_val, treatment_val, ...) rather than leaving X_val, treatment_val, y_val. - The `Sensitivity` helpers are in scope. Their (X, p, treatment, y) is a third argument shape and becomes (X, y, treatment, p). Two fixes the work required: - `timeit` did not use functools.wraps, so `CausalTreeRegressor.bootstrap_pool` reported `(*args, **kw)` to `inspect.signature` and `"wrapped"` as its `__name__` — which is exactly why a signature-based shim could not see it. This also repairs introspection for anything sklearn does with that method. - `v1_order` now reorders only when both `y` and `treatment` are present. Hoisting `y` on its own reached the vendored sklearn tree builders (`build(tree, X, y, ...)`, where it would land ahead of `X`) and `SemiSynthDataGenerator.fit(X, w, y, ...)`. Both are pinned as tests. Tests 43 -> 52. Full suite: 418 passed, 10 skipped. Closes #981. Part of #980. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.
What & why
Kicks off #854 — making CausalML's learners compatible with
sklearn.pipeline.Pipeline, the one intentional breaking change on the v1.0 roadmap (M1).Pipelinecalls the final estimator'sfit(X, y)positionally, but CausalML learners takefit(X, treatment, y, ...)—yis third. In v1.0 the positional order becomes(X, y, treatment, ...).Reordering two required positional arguments can't be done in one step without risking a silent
y/treatmentswap for existing positional callers. So this is a two-step deprecation, and this PR is step one (non-breaking):fit(X, treatment, y)call keeps working exactly as before.treatment/ypositionally now emits aFutureWarningpointing callers to keyword arguments:fit(X, y=y, treatment=treatment). Keyword calls are order-independent, so code that migrates now keeps working unchanged across the v1.0 flip.(X, y, treatment, ...)and delete the deprecation shim #985 on epic Adopt the scikit-learnfit(X, y, treatment, ...)argument order (v1.0 M1) #980.A deprecation window is one-shot: anything whose positional order changes at v1.0 has to warn in this release, or it needs a second cycle of its own. That is the constraint that sets this PR's scope — the warning has to be complete now, even though the flip is still a PR away.
How
causalml/inference/_arg_order.pyholds a re-entrancy-guarded decorator that derives each method's v1.0 order from its own signature:X, theny, thentreatment, with every other parameter keeping its relative position. Each warning names that method's real target order rather than assuming(X, treatment, y, ...), soBaseDRIVLearner.fitreports(X, y, treatment, assignment, ...)andIVRegressor.fitreports(X, y, treatment, w).fit/fit_predict/estimate_ate/predict) and silently missed eleven public methods (Extend the #854 deprecation shim to the remaining publictreatment-before-ymethods #981) — see Scope below.__init_subclass__lives onSerializableLearner, the shared mixin, so one hook covers the meta-learners, the causal/uplift trees and forests, and the IV learners with no per-method edits.functools.wrapspreserves the real signatures for introspection and sklearn metadata routing.DragonNet, Torch/JAXCEVAE,PolicyLearner,TMLELearner,Sensitivity— apply the same decorator at the class level.fit_predict → fit/predict, subclassfit → super().fit(X, treatment, y, *args, **kwargs),estimate_ate → fit_predict → fit— warns at most once per top-level call.metrics/sensitivity.py,dataset/synthetic.py, …) already calls these methods with keywords.Scope
predict, and alsobootstrap,fit_bootstrap_ensemble,bootstrap_pool,prune,fill, and the threeSensitivity.get_*helpers — eleven public methods that a name-based allowlist had missed (Extend the #854 deprecation shim to the remaining publictreatment-before-ymethods #981). Package-wide, nothing whose order changes is left unshimmed, and a test enforces that.predictis in scope. Its current(X, treatment, y, ...)doesn't blockPipeline, sincepredict(X)already works. But flipping onlyfitwould leave v1.0 with the same two arguments in opposite positional orders on one class —fit(X, y, treatment)besidepredict(X, treatment, y)— with noTypeErrorto catch a swap. Methods that don't take bothtreatmentandy, such asBaseRLearner.predict, are left untouched.CausalTreeRegressorand the forests already inherit sklearn'sBaseDecisionTree/ForestRegressorand overridefit(X, y, ...)withfit(X, treatment, y, ...). Leaving them out would ship v1.0 with the most sklearn-shaped classes keeping the least sklearn-shaped signature.UpliftTreeClassifier's validation triple is reordered in place (epic Adopt the scikit-learnfit(X, y, treatment, ...)argument order (v1.0 M1) #980, open question 1), so it stays consistent with the main one:(X, y, treatment, X_val, y_val, treatment_val, ...).Sensitivityhelpers are in scope (epic Adopt the scikit-learnfit(X, y, treatment, ...)argument order (v1.0 M1) #980, open question 2). Theirs is a third argument shape —(X, p, treatment, y)— and becomes(X, y, treatment, p).Pipelinestep —treatmentstill needs to be threaded via sklearn metadata routing, tracked as sklearn metadata routing: lettreatmentreach a learner inside a Pipeline #986. CausalML is not compatible with Scikit-Learn Pipeline #854 stays open until that lands.tests/to keywordfit/predictcalls (917 FutureWarnings) #982 and Migrate docs notebooks and guides to keywordfit/predictcalls #983 — those call sites are also what would break at the flip.Two incidental fixes
Both were required to make the above work, and both are visible in the diff:
causalml/inference/tree/utils.py::timeitnow usesfunctools.wraps. Without it,CausalTreeRegressor.bootstrap_pool— its only user — reported(*args, **kw)toinspect.signatureand"wrapped"as its__name__, which is exactly what hid it from a signature-based shim. Pre-existing bug; it also means anything introspecting that method, sklearn included, was seeing the wrong signature.v1_orderreorders only when bothyandtreatmentare present. Hoistingyon its own reached the vendored sklearn tree builders (build(tree, X, y, ...), whereywould land ahead ofX) andSemiSynthDataGenerator.fit(X, w, y, ...), whose treatment vector is namedw. Both are pinned as regression cases.Tests
tests/test_fit_arg_order.py(52 cases): positional-use warning across S/T/X/R/DR and the classifier variants, keyword silence, single-warning delegation (fit_predict/estimate_ate/ subclasssuper().fit), signature preservation, positional == keyword equivalence (guards against a silenty/treatmentswap),predictwarning/silence,bootstrapwarning, the v1.0 ordering rule itself (DRIV'sassignment, the uplift validation triple, theSensitivityshape, and the two cases that must not move), and behaviour onCausalTreeRegressor/IVRegressor.Two completeness guards walk the installed package: one fails if any public method's order would change without the shim, the other fails if coverage regresses to a fixed list of method names. They cover the optional backends only in the lanes where those backends are installed.
Full local run: 418 passed, 10 skipped.
Refs #854. Part of epic #980; closes #981.