data: fix Marginal.visualize crash on categorical features (#119)#666
Merged
paulbkoch merged 1 commit intoMay 12, 2026
Merged
Conversation
…ml#119) Marginal(...).explain_data(...).visualize(key) raised TypeError: unsupported operand type(s) for -: 'str' and 'str' whenever feature_types[key] was nominal/ordinal because the histogram path unconditionally computed density_dict["names"][1] - density_dict["names"][0] for the bin size, even though for categorical features the names list holds string category labels rather than numeric bin edges. Compute bin_size lazily only on the numeric branch, and apply the same guard on the response side so that a categorical response (e.g. classification target) also renders without crashing. A small helper _is_numeric_bin_edges keeps the predicate explicit. Add tests/data/test_marginal_visualize.py with four cases covering the two regression branches plus the previously-working continuous and overall paths to lock in the contract. Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: jbbqqf <jbaptiste.braun@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #666 +/- ##
==========================================
+ Coverage 66.86% 67.18% +0.32%
==========================================
Files 77 77
Lines 11724 11729 +5
==========================================
+ Hits 7839 7880 +41
+ Misses 3885 3849 -36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Collaborator
|
Very nice PR. Thanks @jbbqqf! |
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.
Summary
Marginal(...).explain_data(...).visualize(key)crashed withTypeError: unsupported operand type(s) for -: 'str' and 'str'wheneverfeature_types[key]was"nominal"or"ordinal", making the marginaldata explainer unusable on every dataset that contained a categorical
feature.
Fixes #119 — visualize has a bug for categorical features
Context
MarginalExplanation.visualizebuilds a small two-panel plotly figure(feature histogram + response histogram). It computed the histogram
bin width for both panels with a single line at the top of the function:
For continuous features
density_dict["names"]is the numeric outputof
np.histogram(...)(bin edges), so the subtraction is fine. Fornominal/ordinal features the same field stores string category labels
returned by
np.unique(...), and the subtraction blows up before anybranch on
is_categoricalever executes — so neither path renders.There is a second instance of the same pattern further down for the
response axis (
resp_bin_size = density_dict["names"][...]— note thatit even pointed at the feature density rather than the response
density). Both are addressed in this PR.
The fix matches the long-standing report from issue #119, with one
small addition: classification targets (categorical response) now also
render, because the response side is now guarded the same way.
Changes
python/interpret-core/interpret/data/_response.pybin_sizeinto theelsebranch so it is only computed whendensity_dict["names"]are numeric bin edges.categorical (e.g. classification target) build a plotly
Histogramwithout
ybins, otherwise computeresp_bin_sizefromresp_density_dict["names"](was previously computed off thefeature density — a latent bug the same line fixed).
_is_numeric_bin_edgespredicate so the categoricalresponse check stays self-documenting; comment cites issue visualize has a bug for categorical features #119
so future readers don't have to re-derive the reasoning.
python/interpret-core/tests/data/test_marginal_visualize.py(new)feature (non-regression), nominal feature on classification target,
and the
key=Noneoverall path.origin/mainwith the exactTypeErrorcited in visualize has a bug for categorical features #119; all four pass on this branch.Reproduce BEFORE/AFTER yourself (copy-paste)
What I ran locally
python -m pytest -vv tests/data/test_marginal_visualize.py→ 4 passedon this branch.
origin/main→ 2 failed, 2 passed (the twofailures are the exact
TypeErrorcited in visualize has a bug for categorical features #119; the passing onescover the continuous and overall paths that were already correct).
ruff check interpret-core/interpret/data/_response.py→ 6 errorsreported, identical to
origin/main(all pre-existingPLC0415/NPY002flags in this file). My change introduces zeronew lint findings.
ruff format --checkon the touched files → already formatted.The selenium / dashboard / full visual suites need extra optional
dependencies (
requests,dash, etc.) that aren't part of thetestingextra; they were not run.Edge cases tested
feature_types=["nominal","continuous"], key=0test_visualize_nominal_feature_continuous_target_succeedstest_visualize_continuous_feature_still_works[0,1,0,1,0,1,0,1], key=0test_visualize_nominal_feature_classification_targetkey=None)test_visualize_overall_unaffectedRisk / blast radius
The change is local to
MarginalExplanation.visualize. It only addscode paths that were previously unreachable (categorical features /
classification responses). Continuous-feature behaviour is byte-equal
to before — same plotly trace, same
xbinsdict. The new_is_numeric_bin_edgeshelper is module-private. No public APIchanges.
Release note
PR drafted with assistance from Claude Code. The change was reviewed
manually against
interpretml/interpret's source and the cited issue.The reproducer block above was used during development; it is the same
one a reviewer can paste verbatim.