-
Notifications
You must be signed in to change notification settings - Fork 1.3k
MAINT Replace stats.mode
calls with fixes._mode
#938
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
Conversation
Codecov ReportBase: 92.62% // Head: 93.87% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #938 +/- ##
==========================================
+ Coverage 92.62% 93.87% +1.24%
==========================================
Files 94 95 +1
Lines 6361 6369 +8
Branches 886 887 +1
==========================================
+ Hits 5892 5979 +87
+ Misses 395 308 -87
- Partials 74 82 +8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Compat with scikit-learn
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.
This is the fix that I did as well in scikit-learn.
I will just postpone it until when all other tests are passed.
The lint failure is new. It looks like |
I fixed it in my PR. Basically, we don't do |
imblearn/utils/fixes.py
Outdated
from sklearn.externals._packaging.version import parse as parse_version | ||
import scipy | ||
import scipy.stats | ||
|
||
|
||
sp_version = parse_version(scipy.__version__) | ||
|
||
|
||
# TODO: Remove when SciPy 1.9 is the minimum supported version | ||
def _mode(a, axis=0): | ||
if sp_version >= parse_version("1.9.0"): | ||
return scipy.stats.mode(a, axis=axis, keepdims=True) | ||
return scipy.stats.mode(a, axis=axis) |
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.
I found the earliest sklearn.utils.fixes._mode
in scikit-learn==1.1.2
:
pip install scikit-learn==1.1.2
python -c 'import sklearn; print(sklearn.utils.fixes._mode)'
We could add the checks for scikit-learn>=1.1.2
like follows (but I think the current fix is simpler and basically does the same thing):
from sklearn.externals._packaging.version import parse as parse_version
import scipy
import scipy.stats
import sklearn
sk_version = parse_version(sklearn.__version__)
sp_version = parse_version(scipy.__version__)
def _mode(a, axis=0):
if sp_version >= parse_version("1.9.0"):
# TODO: Remove when SciPy 1.9 is the minimum supported version
return scipy.stats.mode(a, axis=axis, keepdims=True)
elif sk_version >= parse_version("1.1.2"):
# TODO: Remove when scikit-learn 1.2 is the minimum supported version
return sklearn.utils.fixes._mode(a, axis=axis)
return scipy.stats.mode(a, axis=axis)
Everything is green (e.g. ec50f5b) if we're more restrictive with minimum versions. These still fail: SKLEARN_MIN_VERSION = "1.1.0"
JOBLIB_MIN_VERSION = "1.0.0" ... due to treating
Because A possible fix would be to drop the MRE:pip install scikit-learn==1.1.0 numpy==1.23.5 scipy==1.9.3 joblib==1.2.0 pytest
pytest -Werror::FutureWarning -k 'test_condensed_nearest_neighbour' imblearn/ |
I modify a couple of things that I merge in |
stats.mode
calls with fixes._mode
stats.mode
calls with fixes._mode
Thanks @hayesall. This should make |
Reference Issue
Fix #937
What does this implement/fix? Explain your changes.
I caught up on scikit-learn/scikit-learn#23633 and replaced the
scipy.stats.mode
calls withsklearn.utils.fixes._mode
calls.JOBLIB_MIN_VERSION = "1.1.1"
SKLEARN_MIN_VERSION = "1.1.3"
NUMPY_VERSION = "1.21.0"
inpy37_conda_defaults_openblas
build (otherwise conda runs into a large number of incompatible packages).Similar to the notes there, this should be replaced when scipy 1.9 is the minimum version:
https://github.com/scikit-learn/scikit-learn/blob/7ec1bfc2ab7425bcd984d631b5bfeb9082ce11bf/sklearn/utils/fixes.py#L168-L172