Skip to content

Add CatBoost documentation and test coverage for Explainer#924

Open
aman-coder03 wants to merge 2 commits into
uber:masterfrom
aman-coder03:feature/explainer-catboost
Open

Add CatBoost documentation and test coverage for Explainer#924
aman-coder03 wants to merge 2 commits into
uber:masterfrom
aman-coder03:feature/explainer-catboost

Conversation

@aman-coder03

@aman-coder03 aman-coder03 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

this pr adds documentation and test coverage for using CatBoostRegressor as the model_tau estimator in the Explainer class when method="auto"

what changed

  • updated the model_tau docstring to mention catboost support
  • added a regression test using CatBoostRegressor, guarded with pytest.importorskip("catboost"), to verify that Explainer(method="auto") works correctly and returns feature importances with the expected shape

why

while investigating issue #826, i verified that current versions of catboost already expose the feature_importances_ attribute after fitting, so Explainer(method="auto") works without any implementation changes
this pr adds documentation and a regression test to ensure catboost support remains covered and continues to work as expected

closes #826

Types of changes

What types of changes does your code introduce to CausalML?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc. This PR template is adopted from appium.

@jeongyoonlee jeongyoonlee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you reframe this PR as test + docs only?

The underlying bug doesn't reproduce on current CatBoost. CatBoostRegressor.feature_importances_ has been a class-level property since ~catboost 0.15 (2019), and check_conditions() fits the dummy model before the hasattr assertion (explainer.py:97-103) — so the original assertion already passes for CatBoost and default_importance() reads .feature_importances_ fine. Running the PR's own scenario on master (without this PR's code):

exp = Explainer(method="auto", model_tau=CatBoostRegressor(iterations=10, verbose=0), ...)
exp.get_importance()   # passes: pd.Series, len == n_features, normalized to 1

It passes, so test_explainer_auto_importance_catboost is green with and without the change, and the get_feature_importance() fallback is never reached for any of sklearn/lightgbm/xgboost/catboost (all satisfy feature_importances_ first) — it's dead code.

Please drop the _get_feature_importances helper and its fallback, keeping just the (genuinely additive) CatBoost-as-model_tau test and the docstring update. Also update the title/description away from "Bugfix" so the history doesn't record a fix for a non-bug.

@aman-coder03 aman-coder03 changed the title Support CatBoost feature importances in Explainer Add CatBoost coverage to Explainer tests and documentation Jul 4, 2026
@aman-coder03 aman-coder03 changed the title Add CatBoost coverage to Explainer tests and documentation Add CatBoost documentation and test coverage for Explainer Jul 4, 2026
@aman-coder03 aman-coder03 requested a review from jeongyoonlee July 4, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allowing CatBoost for model_tau input to Explainer class

2 participants