DistrictDataLabs / yellowbrick

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

Allow marker style to be specified in Validation Curve #1258

Closed lwgray closed 2 years ago

lwgray commented 2 years ago
  1. Summarize your PR (HINT: See CHECKLIST/TEMPLATE below!) -->

This PR addresses StackOverflow question regarding how to change the marker of Validation Curve. https://stackoverflow.com/questions/67267401/how-to-change-the-marker-in-yellowbrick-plots

I have made the following changes:

  1. Added marker argument to both ValidationCurve and quickmethod

Sample Code and Plot (Original [Top], New [Bottom])

import numpy as np

from yellowbrick.datasets import load_energy
from yellowbrick.model_selection import ValidationCurve

from sklearn.tree import DecisionTreeRegressor

# Load a regression dataset
X, y = load_energy()

viz = ValidationCurve(
    DecisionTreeRegressor(), param_name="max_depth",
    param_range=np.arange(1, 11), cv=10, scoring="r2",
    markers="-v"
)

# Fit and show the visualizer
viz.fit(X, y)
viz.show()

imageimage

TODOs and questions

Still to do:

Questions for the @DistrictDataLabs/team-oz-maintainers:

CHECKLIST

codecov[bot] commented 2 years ago

Codecov Report

Merging #1258 (f1f30c4) into develop (5d54821) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1258   +/-   ##
========================================
  Coverage    90.75%   90.75%           
========================================
  Files           93       93           
  Lines         5300     5301    +1     
========================================
+ Hits          4810     4811    +1     
  Misses         490      490           
Impacted Files Coverage Δ
yellowbrick/model_selection/validation_curve.py 98.14% <100.00%> (+0.03%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

lwgray commented 2 years ago

@rebeccabilbro where are you seeing the annotation error? During building the docs????

rebeccabilbro commented 2 years ago

@rebeccabilbro where are you seeing the annotation error? During building the docs????

@lwgray look in your PR diff in the “files changed” view, all the way at the bottom.

lwgray commented 2 years ago

@rebeccabilbro I am unsure how to resolve the umap annotation check. Any suggestions?

rebeccabilbro commented 2 years ago

@lwgray ah ok - have you already tried updating the umap version in docs/requirements.txt as suggested above?

lwgray commented 2 years ago

docs/requirements doesn't have umap

rebeccabilbro commented 2 years ago

docs/requirements doesn't have umap

That’s probably the problem :)

lwgray commented 2 years ago

@rebeccabilbro Adding umap to requirements doesn't resolve the issue....It is specific to this plot directive for umap quick-method as other umap Visualizer plot directives appear to run without issue

rebeccabilbro commented 2 years ago

FYI @lwgray this has been approved but left unmerged for over a month

lwgray commented 2 years ago

@rebeccabilbro I didn't merge because i couldn't resolve the annotation error you brought up. @bbengfort can you assist us?

rebeccabilbro commented 2 years ago

@lwgray Benjamin is on vacation. I'm going to merge this once the tests pass with @pdamodaran's updates. In the future, please merge your own PRs once you've received an approval.

rebeccabilbro commented 2 years ago

Unfortunately @lwgray it looks like you have a bit of new work to do to get all the image comparison checks to pass. Hopefully this won't take too much work on your part!