TST Extend tests for scipy.sparse.*array in sklearn/feature_selection/tests/test_feature_select.py#27239
Conversation
glemaitre
left a comment
There was a problem hiding this comment.
Actually, the implementation of r_regression is not compatible. I have to check the reason.
glemaitre
left a comment
There was a problem hiding this comment.
You need to edit the file sklearn/feature_selection/_univariate_selection.py in l.326 and replace the code from:
if issparse(X):
X_means = X.mean(axis=0).getA1()
else:
X_means = X.mean(axis=0)to
if isspmatrix(X):
X_means = X.mean(axis=0).getA1()
else:
X_means = X.mean(axis=0)You should import isspmatrix from scipy.sparse
a18e73c to
180013a
Compare
|
Thanks for fix! |
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
glemaitre
left a comment
There was a problem hiding this comment.
I fixed the linting problem by removing the useless import. I also added an entry in the changelog at the same time since we are modifying some public facing code.
LGTM otherwise. @OmarManzoor feel free to have a look at this one.
doc/whats_new/v1.4.rst
Outdated
|
|
||
| - |Enhancement| :func:`metrics.f_regression` and :func:`metrics.r_regression` now | ||
| supports SciPy sparse arrays. | ||
| :pr:`27239` by :user:`Tialo <Tialo>`. |
There was a problem hiding this comment.
Feel free to update your name (only the first mention since the second one link to your GitHub handle). This is also fine keeping it as-is.
OmarManzoor
left a comment
There was a problem hiding this comment.
Thanks @Tialo for the PR.
OmarManzoor
left a comment
There was a problem hiding this comment.
I made the minor changes. LGTM.
|
Thanks! I changed my name, ready to merge |
|
Oh apparently there is a new deprecation warning. I made the changes. We should be good to go. |
…ion/tests/test_feature_select.py` (scikit-learn#27239) Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com> Co-authored-by: Omar Salman <omar.salman@arbisoft.com>
Reference Issues/PRs
Towards #27090.
What does this implement/fix? Explain your changes.
Any other comments?