[MRG] FIX make count_nonzero dtype invariant wrt axis#12341
[MRG] FIX make count_nonzero dtype invariant wrt axis#12341qinhanmin2014 merged 2 commits intoscikit-learn:masterfrom
Conversation
|
My intention is that this is merged before that is. Or the other way. If I
removed it there, tests would fail.
https://github.com/numpy/numpy/blob/7cd94f262b1b12d29f7903195098903cd0e9db22/numpy/core/src/multiarray/compiled_base.c#L173
is where I got the information that it is intp :|
|
|
@qinhanmin2014, please confirm this works on your window setup too :) |
qinhanmin2014
left a comment
There was a problem hiding this comment.
LGTM, these new test passes (and fail on master) on my Windows.
I'm not familiar with numpy, so not sure whether the return type will be changed since it's not officially documented. Maybe I'll cast all the return values to a specific type (e.g., int), but I guess this is not a big problem here.
|
And let's hurry this one into 0.20.1 unless you oppose. |
|
I wouldn't even ordinarily publicly document this change, as I don't expect
users care about it. I wouldn't push for it to be in 0.20.1.
|
|
Apologies for the previous comment. I somehow messed up some PRs. |
rth
left a comment
There was a problem hiding this comment.
LGTM apart for the comment below. Thanks!
|
|
||
| # Check dtypes with large sparse matrices too | ||
| X_csr = sp.csr_matrix((X_csr.data, X_csr.indices.astype(np.int64), | ||
| X_csr.indptr.astype(np.int64)), shape=X_csr.shape) |
There was a problem hiding this comment.
If you want to test this, you would have to monkeypatch X_csr.indices etc as otherwise csr_matrix will downcast them as needed (here to int32)
qinhanmin2014
left a comment
There was a problem hiding this comment.
I think the test failures are unrelated.
…t-learn#12341)" This reverts commit 88744dc.
…t-learn#12341)" This reverts commit 88744dc.
Pulled out of #11179, where using
samplewise=Truechanged the confusion matrix dtype.I don't think this should affect any public interfaces until #11179.