Closed StepanTita closed 1 year ago
Thanks @StepanTita for this PR. I managed to reproduce it. It was a little confusing first because the tree structures were different, but this was because of different tree index values.
Do we still want to display the nodes/leaves which are not part from the new dataset (those one from simple oval shapes) ?
I think we should fix it also for regression trees... right ?
Thanks @StepanTita for this PR. I managed to reproduce it. It was a little confusing first because the tree structures were different, but this was because of different tree index values.
Do we still want to display the nodes/leaves which are not part from the new dataset (those one from simple oval shapes) ?
I think we should fix it also for regression trees... right ?
Well, I believe we still need to draw empty nodes because they form the tree structure, otherwise it would just be blank space right?
Regarding the regression trees, I tried to reproduce this issue, and then double checked it and this is not a problem there: https://github.com/parrt/dtreeviz/blob/a0e85a0d3f64ec0616dcfafb5fcf72f0cbf434b8/dtreeviz/trees.py#L1373
This would just be an empty array, which later would lead to nan for mean, but it will still plot empty plot, will not throw an error.
@StepanTita sorry for the late response :)
Indeed, letting them as empty nodes I think it would be a good option.
I tried to see how the tree is looking when fancy=True (and with another dataset than training) and it will raise some exception when the split node will have no data.
looks like a merge conflict?
Close in favor of https://github.com/parrt/dtreeviz/pull/307
Fixes the bug of graphviz erroring out due to file not found Mentioned in that issue: https://github.com/parrt/dtreeviz/issues/298
Code to reproduce:
Error message:
Colab example of failing: https://colab.research.google.com/drive/1TTX4m7H-S1y5BMqKy_YcWzmlaqJjYkn9?usp=sharing
Colab example of working after the fix: https://colab.research.google.com/drive/1xxPYYAKNwvkcF4Yj6cLK6fUJGxz-W0j6?usp=sharing
Rendered tree after fix:
It might be worth adding some kind of a warning message, but I couldn't find anything like that across the package, so decided not to add it myself.