Skip to content
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 VIF add a new function 'feature_selection_vif' #3366

Closed
wants to merge 5 commits into from

Conversation

pydemia
Copy link

@pydemia pydemia commented Jan 8, 2017

  1. Fix 'variance_inflation_factor' in stats.outliers_influence : Apply sm.add_constant process for proper results.
  2. Add 'feature_selection_vif' in stats.multivariate_tools : A Function of Stepwise Feature Elimination with VIF

1. Fix 'variance_inflation_factor' in stats.outliers_influence :  Apply sm.add_constant process for proper results.
2. Add 'feature_selection_vif' in stats.multivariate_tools : A Function of Stepwise Feature Elimination with VIF
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0002%) to 88.026% when pulling be05459 on pydemia:pydemia into 22f2766 on statsmodels:master.

@pydemia
Copy link
Author

pydemia commented Jan 8, 2017

The Travis CI throws 2 errors on Python 2.6, which are irrelevant to my commit :

  1. KeyError: 'Canonical Correlation'
    ERROR: statsmodels.multivariate.tests.test_cancorr.test_cancorr

  2. KeyError: 'Wilks\xe2\x80\x99 lambda'
    ERROR: statsmodels.multivariate.tests.test_manova.test_manova_sas_example
    ERROR: statsmodels.multivariate.tests.test_manova.test_manova_test_input_validation
    ERROR: statsmodels.multivariate.tests.test_multivariate_ols.test_glm_dogs_example
    ERROR: statsmodels.multivariate.tests.test_multivariate_ols.test_specify_L_M_by_string
    ERROR: statsmodels.multivariate.tests.test_multivariate_ols.test_from_formula_vs_no_formula

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0002%) to 88.026% when pulling 176d5bf on pydemia:pydemia into 22f2766 on statsmodels:master.

@coveralls
Copy link

coveralls commented Jan 8, 2017

Coverage Status

Coverage decreased (-5.2%) to 82.853% when pulling bba4762 on pydemia:pydemia into 22f2766 on statsmodels:master.

@josef-pkt
Copy link
Member

Sorry for the slow response. I need to think about this and it's too far away from what I'm currently looking at.

e.g. adding the sm.add_constant is not backwards compatible and will most likely break current use cases.

@josef-pkt
Copy link
Member

The feature_selection_vif looks fine in general. There should be no print in the code.

Unit tests are missing.

@josef-pkt
Copy link
Member

This pull request introduces 1 alert when merging 176d5bf into 22f2766 - view on lgtm.com

new alerts:

  • 1 for Variable defined multiple times

Comment posted by lgtm.com

@pydemia
Copy link
Author

pydemia commented May 2, 2018

I should apply your mentions and retry the pull request.

@pydemia pydemia closed this May 2, 2018
@jseabold jseabold removed this from the 0.10 milestone Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants