Open icfly2 opened 3 years ago
This update will break compatibility with scikit-learn < 0.20, so it would be necessary to update the requirements and setup.py to require at least this version (eli5 currently allows >=0.18).
Is the upgrade of the sklearn requirement acceptable, given that 0.18 is now 4 years old and 0.24 has a RC out?
Merging #397 (805a3c1) into master (017c738) will decrease coverage by
16.74%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #397 +/- ##
===========================================
- Coverage 97.32% 80.58% -16.75%
===========================================
Files 49 49
Lines 3142 3142
Branches 585 585
===========================================
- Hits 3058 2532 -526
- Misses 44 569 +525
- Partials 40 41 +1
Impacted Files | Coverage Δ | |
---|---|---|
eli5/sklearn/permutation_importance.py | 93.68% <100.00%> (-6.32%) |
:arrow_down: |
eli5/keras/gradcam.py | 0.00% <0.00%> (-100.00%) |
:arrow_down: |
eli5/xgboost.py | 4.26% <0.00%> (-95.13%) |
:arrow_down: |
eli5/formatters/image.py | 5.47% <0.00%> (-94.53%) |
:arrow_down: |
eli5/keras/explain_prediction.py | 4.81% <0.00%> (-91.57%) |
:arrow_down: |
eli5/lightgbm.py | 4.27% <0.00%> (-90.60%) |
:arrow_down: |
eli5/catboost.py | 11.53% <0.00%> (-88.47%) |
:arrow_down: |
eli5/_decision_path.py | 35.48% <0.00%> (-64.52%) |
:arrow_down: |
eli5/keras/__init__.py | 50.00% <0.00%> (-50.00%) |
:arrow_down: |
eli5/ipython.py | 85.71% <0.00%> (-14.29%) |
:arrow_down: |
... and 5 more |
Is the upgrade of the sklearn requirement acceptable, given that 0.18 is now 4 years old and 0.24 has a RC out?
I'd say that yes, it is.
Thanks for the PR 👍
/eli5/sklearn/text.py in
----> 4 from sklearn.feature_extraction.text import VectorizerMixin # type: ignore
should be renamed to _VectorizerMixin
~/.local/lib/python3.8/site-packages/eli5/sklearn/transform.py ----> 6 from sklearn.feature_selection.base import SelectorMixin # type: ignore
from sklearn.feature_selection import SelectorMixin
@veonua I'm a bit confused what you are suggesting, master reads:
try:
from sklearn.feature_extraction.text import _VectorizerMixin as VectorizerMixin
except ImportError: # Changed in scikit-learn 0.22
from sklearn.feature_extraction.text import VectorizerMixin
your other comment is also already addressed in this PR
can you please publish these changes to pip?
Thanks you! This were merged in eli5-org/eli5#2 and released to PyPI with v0.11
Change as per FutureWarning in sklearn, to allow eli5 to be used with sklearn 0.24 and newer. docs
Currently issues FutureWarning