earthlab / matplotcheck

A python package for checking and testing matplotlib plots. We use this for autograding student assignments but there are many other potential use cases including package testing (for packages with plots)!
https://matplotcheck.readthedocs.io
BSD 3-Clause "New" or "Revised" License
18 stars 8 forks source link

X-limits issue with base.assert_line() and seaborn.regplot() #235

Closed marty-larocque closed 4 years ago

marty-larocque commented 4 years ago

Currently, the documentation for base.assert_line(), reads: "Asserts that there exists a line [that]... goes at least from x coordinate min_val to x coordinate max_val".

More simply, this means that base.assert_line() is checking that the line extends to the limits of the x-data. It is checking that the line extends to the left of the lowest x-value and to the right of the highest x-value.

This can cause an issue when used with seaborn.regplot(). I have attached a plot that is the result of using seaborn.regplot() where the plot appears to be 'cropped in'. The x-limits for the plot partially cut off the outside x-values. (It should be noted that the data for the regression line does not extend past the x-limits of the plot. There isn't any more invisible line past the edges of the plot.)

mygraph

What this means is that the x-values for the regression do not quite span the same range as the input data. Therefore, if this issue occurs, the assertion will always fail, even if the slope and y-intercept are correct.

This can also be seen on the assignment ea-python-2020-04-lidar-uncertainty.ipynb.

We should change the behavior of base.assert_line() so that this issue does not occur. Likely, this would mean that base.assert_line() would no longer check the min and max x-values of the regression line.

lwasser commented 4 years ago

nathan is working on this here: #248 once we merge this we can add this to @ryla5068 pr's above and add x/y lims as a parameter!

marty-larocque commented 4 years ago

@lwasser this can be closed when #239 is merged.

nkorinek commented 4 years ago

This can be closed, fixed in #274 @lwasser

lwasser commented 4 years ago

holy cow - closing this too thank you!