[MRG+2] Fix for issue #6352#6376
Conversation
sklearn/tree/export.py
Outdated
| sorted_values = sorted(value, reverse=True) | ||
| alpha = int(np.round(255 * (sorted_values[0] - sorted_values[1]) / | ||
| alpha = 0 if len(sorted_values) == 1 else int(np.round(255 * (sorted_values[0] - sorted_values[1]) / | ||
| (1 - sorted_values[1]), 0)) |
There was a problem hiding this comment.
This is too long and not readable. Just use a regular multiline if / else instead of a ternary expression.
|
Other than the style comment, LGTM. |
|
Hi, @ogrisel. I think, everything is done. |
|
LGTM +1. (You can prefix your PR with [MRG+2]) |
| out = StringIO() | ||
| export_graphviz(clf, out_file=out, filled=True) | ||
| contents1 = out.getvalue() | ||
| contents2 = 'digraph Tree {\n' \ |
There was a problem hiding this comment.
Note from PEP8:
The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation.
For long strings you can do:
contents2 = ('digraph Tree {\n'
'node ...\n'
'0 [label ...]\n'
'}')Alternatively in your particular case, this would work:
contents2 = '\n'.join(['digraph Tree{', 'node ...', '0 [label ...', '...'])There was a problem hiding this comment.
I've used this line separator because of consistency. All other lines in this file divided with backslash. I think in this case I should replace all backslashes.
There was a problem hiding this comment.
So. Keep it with backslashes? Or apply PEP8 recommendation to all strings?
There was a problem hiding this comment.
Keep it with backslashes is fine, that's what I meant.
|
@TomDLT merge ? |
|
I hope you will merge before somebody pushes some changes to this files in master =) This little fix take a lot of time. |
This is a fix and tests for issue #6352, which allows to generate fill colors for degraded learning set.