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

BUG: Fixes axes limit for PredictionError plot #1193 #1208

Closed pkaf closed 2 years ago

pkaf commented 2 years ago

This fixes Issue #1193. X and Y axis limits are synced and set to overall max/min values of (X, Y).

Screen Shot 2022-01-20 at 9 16 58 am

bbengfort commented 2 years ago

@pkaf thank you again for contributing to Yellowbrick! Since this is a pretty simple change; I think this will work well - the tests are failing because the baseline images are now incorrect; I'm going to regenerate the images and spot-check a few of them. I'll let you know when I do so that you can also view the new baseline images. Note that this means I need write access to your fork - I'll let you know if you need to do anything extra.

pkaf commented 2 years ago

@bbengfort may be the _sharedlimits is worth keeping with True as a default value. With following code modification inside the draw function?

self.ax.set_xlim(min(min(y), min(y_pred)), max(max(y), max(y_pred)))
self.ax.set_ylim(self.ax.get_xlim())

to

if self.shared_limits is True:
    self.ax.set_xlim(min(min(y), min(y_pred)), max(max(y), max(y_pred)))
    self.ax.set_ylim(self.ax.get_xlim())

This will make the current test in-place still relevant. Suggestion?

codecov[bot] commented 2 years ago

Codecov Report

Merging #1208 (33ca4af) into develop (838063e) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1208   +/-   ##
========================================
  Coverage    90.42%   90.42%           
========================================
  Files           90       90           
  Lines         5096     5097    +1     
========================================
+ Hits          4608     4609    +1     
  Misses         488      488           
Impacted Files Coverage Δ
yellowbrick/regressor/prediction_error.py 93.54% <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 838063e...33ca4af. Read the comment docs.

bbengfort commented 2 years ago

@pkaf that works for me - looks like the tests are passing; I'll handle the merge conflicts and we'll get this merged! Thanks again for your patience.