-
Notifications
You must be signed in to change notification settings - Fork 256
Fix/pd variance plot test coverage #820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/pd variance plot test coverage #820
Conversation
…ailed before because of simple explanation values.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #820 +/- ##
==========================================
+ Coverage 76.37% 81.05% +4.67%
==========================================
Files 73 73
Lines 8477 8475 -2
==========================================
+ Hits 6474 6869 +395
+ Misses 2003 1606 -397
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
||
@pytest.mark.parametrize('n_axes', [4, 5, 6, 7]) | ||
@pytest.mark.parametrize('n_cols', [2, 3, 4]) | ||
@pytest.mark.parametrize('exp', [lazy_fixture('explanation_importance')]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this needs to be a lazy fixture since it doesn't have it any parameters, can't test functions just depend on this fixture directly as usual?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes ... I think initially I intended to use both types of explanations in the test ... I will update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just not sure we need to use lazy_fixture
at all.
* Fixed minor bug for conditional plots. * Included importance explanation fixture. * Included interaction explanation fixture. * Included test for hbar ncols. Solved flake8 issues. * Included value error ax for hbar plot. * Included interaction plot tests. * Included plot_pd_variance tests. * Updated the explanation objects with random values. Fixed test that failed before because of simple explanation values. * More random values. * Removed lazy fixture.
This PR includes tests for the potting functionality of the
PartialDependenceVariance
explainer to increase coverage (i.e. addresses #760 ).