DistrictDataLabs / yellowbrick

Visual analysis and diagnostic tools to facilitate machine learning model selection.
http://www.scikit-yb.org/
Apache License 2.0
4.29k stars 559 forks source link

Add plot directives #687

Closed rwhitt2049 closed 5 years ago

rwhitt2049 commented 5 years ago

Checklist to go through for (potentially) replacing python code + png import with the plot directive. This is just a project management issue to give contributors some direction.

Quickstart

http://www.scikit-yb.org/en/latest/quickstart.html quickstart.rst

Gallery

… lots to do and figure out here …

update: we've made the gallery independent and created a script to generate the gallery docs (the plot directive didn't work well in the gallery). See #732

Tutorial

update: this has been updated to use the new datasets module (see #818) but we've opted not to do the plot directive because it disrupts the flow of the tutorial and makes the docs take a long time to build

~Lot's of split code here, so I'll need to know how the team wants it formatted~

~http://www.scikit-yb.org/en/latest/tutorial.html# tutorial.rst~

~- [ ] modelselect_linear_svc.png~ ~- [ ] modelselect_nu_svc.png~ ~- [ ] modelselect_svc.png~ ~- [ ] modelselect_sgd_classifier.png~ ~- [ ] modelselect_kneighbors_classifier.png~ ~- [ ] modelselect_logistic_regression_cv.png~ ~- [ ] modelselect_logistic_regression.png~ ~- [ ] modelselect_bagging_classifier.png~ ~- [ ] modelselect_extra_trees_classifier.png~ ~- [ ] modelselect_random_forest_classifier.png~

API

Radviz

Rankd

Pcoords

PCA

Manifold

*update: we have opted not to use the plot directive for manifold because it makes the docs take a long time to build

~- [ ] Features\manifold concrete_tsne_manifold.png (this one has no accompanying code in rst)~ ~- [ ] Features\manifold occupancy_select_k_best_isomap_manifold.png~ ~- [ ] Features\manifold concrete_isomap_manifold.png~

Importances

RFECV

*update: we have opted not to use the plot directive for RFEVC because it makes the docs take a long time to build

~- [ ] Features\rfecv rfecv_sklearn_example.png~ ~- [ ] Features\rfecv rfecv_credit.png~

Joint plot

Note: Joint Plot documentation is updated in #728

Text Visualizers

DispersionPlot

Note that the docs/api/text/images/dispersion_docs.png image was not removed because it is being used in the gallery.

bbengfort commented 5 years ago

Thank you so much for compiling this @rwhitt2049!

rebeccabilbro commented 5 years ago

Per #709 - when we update quickstart.rst to auto-gen the images, let's double check that the 45 degree line appears on the prediction error plot.

rebeccabilbro commented 5 years ago

Per #703 - when we update quickstart.rst to auto-gen the images, let's double-check that we're using fit_transform for the joint plot.

bbengfort commented 5 years ago

@rebeccabilbro for #709 and #703 should we just update the quickstart.rst in this PR rather than in an additional PR?

rebeccabilbro commented 5 years ago

@bbengfort - yep!

bbengfort commented 5 years ago

er ... sorry I thought that was a comment on the review for PR #446 (too many windows open); however, I am happy to make those changes in #446 if that will make the review easier.

rebeccabilbro commented 5 years ago

@bbengfort nah, I say we stick with keeping #446 nice and focused on the POC with the feature importances and k-elbow docs. So excited to be moving forward with this docs refresh!!

bbengfort commented 5 years ago

719 handles the quickstart documentation.

bbengfort commented 5 years ago

Note that the cleanup is going to require the gallery to be updated to use the plot directive relatively soon - the gallery uses many of the static images that we'd like to delete throughout this documentation process!

bbengfort commented 5 years ago

Note - we had to use the :context: close-figs on all of our plot directives to ensure they rendered correctly. I'm still looking into this, but it's something to be aware of as you're working on these files!

rebeccabilbro commented 5 years ago

One of things that @lwgray and @Kautumn06 identified as part of #669 is that when we updated theUMAPVisualizer to use the plot directive, the images don't show up in the ReadtheDocs version (but they do show up when you build locally). The reason for this is that we don't have the main dependency of UMAPVisualizer, an optional dependency umap-learn, in our requirements.txt. Since I don't think we want to require umap-learn, we might have to move back to using static images in the docs for the UMAPVisualizer — unless anyone has another suggestion?

lwgray commented 5 years ago

no other suggestions from me.

rebeccabilbro commented 5 years ago

Going to go ahead and mark this one complete! 🎉 Thanks for all the hard work to everyone who contributed to this massive but very worthwhile effort, with special thanks to @Kautumn06 @lwgray @pdamodaran for reviewing the large majority of the resulting PRs!