Repository navigation
Fix plot_tree crash on truncated and multi-output SGTRegressor nodes - #84
Merged
Merged
Conversation
plot_tree raised TypeError for an SGTRegressor drawn with max_depth, or fitted on a multi-output y (#82). _draw_leaf_text formatted node['value'] as one float, but tree_export sets value=None on internal nodes and a per-output list on multi-output leaves, and plot_tree draws nodes cut off by max_depth as leaves. Classifiers did not crash because class_counts is filled in on internal nodes too, but multi-output classifiers silently showed only output 0's class. Truncated regressor internals now render as "…". Multi-output leaves render one entry per output as "[a, b]" for both regressors and classifiers; plot_tree resolves class names per output (pass one list per output for multi-output classifiers). The per-output and list formatting helpers move out of export_text so both renderers share them. The plot_tree docstring now states that n= is a rounded weighted count for classifiers and an unweighted count for regressors. No test caught this: the only max_depth test used a classifier, and no plot_tree test fitted a 2-D y. Add a regressor test over single and multi-output, with and without max_depth, plus a multi-output classifier test, and the test-checker blind tests promoted for _draw_leaf_text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # sgtlearn/_export.py
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.
Fixes #82.
Root cause
_draw_leaf_textformattednode['value']as one float.tree_exportsetsvalue=Noneon internal regressor nodes and a per-output list on multi-output leaves, andplot_treedraws nodes cut off bymax_depthas leaves, so both cases raisedTypeError. Classifiers didn't crash becauseclass_countsis filled in on internal nodes too, but multi-output classifiers silently showed only output 0's class (andclass_names=Trueproduced garbled labels).Changes
….[a, b], for regressors and classifiers.plot_treeresolves class names per output; pass one list per output for multi-output classifiers (same convention asexport_text).export_text, so both renderers share them.plot_treedocstring states thatn=is a rounded weighted count for classifiers and an unweighted count for regressors. Unifying the two needs a change to the C++ export, which is left out of this PR.Why no test caught it
The only
max_depthtest used a classifier, and noplot_treetest fitted a 2-Dy.Tests
max_depthNone or 1) and a multi-output classifier test intests/test_plot_tree.py. Both fail onmainand pass on this branch._draw_leaf_text, promoted totests/regression/: 9 fail onmainand pass here, 30 pass on both, none fail here.🤖 Generated with Claude Code