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

Better argmax handling in DiscriminationThreshold #1202

Closed falcaopetri closed 3 years ago

falcaopetri commented 3 years ago

Although documentation allows DiscriminationThreshold(model, argmax=None), current implementation raises an exception since it assumes argmax to be a string.

I have made the following changes:

  1. Allow argmax to be None (as already documented)
  2. Raise error in case argmax is an invalid metric (similar to the check of exclude)
  3. Add tests to check argmax
  4. Improve docs of argmax and exclude
  5. Fix typo in CONTRIBUTING.md

CHECKLIST

codecov[bot] commented 3 years ago

Codecov Report

Merging #1202 (8dd58ce) into develop (1852319) will increase coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1202      +/-   ##
===========================================
+ Coverage    90.40%   90.43%   +0.02%     
===========================================
  Files           90       90              
  Lines         5077     5089      +12     
===========================================
+ Hits          4590     4602      +12     
  Misses         487      487              
Impacted Files Coverage Δ
yellowbrick/classifier/threshold.py 98.71% <100.00%> (+0.10%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1852319...8dd58ce. Read the comment docs.