New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update User Guide with Decision Tree Visualization #1678
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1678 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 242 242
Lines 19273 19273
=======================================
Hits 19265 19265
Misses 8 8
Continue to review full report at Codecov.
|
roc_curve, | ||
graph_roc_curve, | ||
graph_confusion_matrix, | ||
binary_objective_vs_threshold, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reordered imports alphabetically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did flake8 recommend this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freddyaboulton Yeah it weirdly did and it came out of nowhere! One day it wasn't recommending it and the next it was. Correct me if I'm wrong but it makes sense right, as far as alphabetization is concerned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorting alphabetically makes it look tidier! But your comment about one day not recommending it and the other day recommending it makes me think your version of flake8 got upgraded? Locally, my make lint
passed before this change got merged.
"cell_type": "markdown", | ||
"metadata": {}, | ||
"source": [ | ||
"Now let's make the pipeline more complex by replacing the Tree with a Forest." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of replacing the entire pipeline, which would have resulted in reduced performance and some uglier graphs, I chose to start with a DecisionTree and then change it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ParthivNaresh Looks great!!
roc_curve, | ||
graph_roc_curve, | ||
graph_confusion_matrix, | ||
binary_objective_vs_threshold, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did flake8 recommend this?
@@ -433,6 +473,13 @@ | |||
" include_shap_values=True, output_format=\"dict\")\n", | |||
"print(json.dumps(report, indent=2))" | |||
] | |||
}, | |||
{ | |||
"cell_type": "code", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit-pick: Maybe we should get rid of the empty cell?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Output looks good! Just added a comment to change wording for clarity :)
https://feature-labs-inc-evalml--1678.com.readthedocs.build/en/1678/user_guide/model_understanding.html#Tree-Visualization
"cell_type": "markdown", | ||
"metadata": {}, | ||
"source": [ | ||
"Now let's make the pipeline more complex by replacing the Tree with a Forest." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bahaha nice! Can we update this to be more specific ex: the Decision Tree pipeline with a Random Forest pipeline or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also... maybe rather than just stating we'll make it more complex (because why?), maybe we should just introduce it in the next part? I guess as a user, I don't understand what the point is :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc changes look good!
Fixes #1541