-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
TST Extend tests for scipy.sparse.*array
in sklearn/utils/tests/test_sparsefuncs.py
#27242
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
sp.hstack( | ||
[ | ||
csr_container(np.full((13, 1), fill_value=np.nan)), | ||
sp.random(13, 1, density=0.8, random_state=42), | ||
], | ||
format="csr", | ||
), | ||
) | ||
for csr_container in CSR_CONTAINERS |
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.
Due to the call to sp.hstack
, we whill always return a sparse matrix.
The random in the previous tuple is also only testing sparse matrices.
One potential solution is to create sparse matrices, and parametrize with CSR_CONTAINER
and intenally cast X1
and X2
:
X1 = csr_container(X1)
X2 = csr_container(X2)
Otherwise, LGTM for the rest. |
@glemaitre I removed csr_container in first part of parameterisation and left just np.full if it becomes sparse matrix anyway |
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. Thanks @Tialo.
Another ready @OmarManzoor
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. Thanks @Tialo
…st_sparsefuncs.py` (scikit-learn#27242)
Reference Issues/PRs
Towards #27090.
What does this implement/fix? Explain your changes.
Any other comments?
In tests like
test_mean_variance_axis0
I checked if*array
will work with*matrix
. If you think that we should check matrices only with matrices, same for arrays, I can reformat parameterisations usingzip