In general the good looks good, but I would like to know the exact problem that you are trying to solve. Ideally, this problem should be illustrated by a test that fails without your patch and passes with it. The idea being that it will guide later refactoring.

In the long run, we want to tackle the issue of multiple types in Cython using fused types.

I found that computing cosine distance fails on 32-bit floating point sparse arrays. Added regression tests.

I added some clarifying documentation on the assign_rows_csr but perhaps this should fail given it's supposed to be no-copy?

Fused types would be nice.

This fix should work, but I'm afraid that this is equivalent to changing the dtype of the sparse matrix prior and passing it to the function as it produces a copy.

It would be great if you could add fused type support so that we can do this without copying ! Just for this function would be a great start.

I had a quick look at this today - the code looks fine to me, and the fix works. I would recommend merging it anyway - but fused types needs done too.

inplace_csr_row_normalize_l1 and inplace_csr_row_normalize_l2 are supposed to be inplace operations, therefore they cannot by design make a copy of the input data.

The only correct fix for those 2 functions is to use cython fused types for np.float32 and np.float64 (or directly cython.floating that should work for this case AFAIK).

Therefore I am -1 on this part of the PR.

+0 for the change to assign_rows_csr if a test is added, but I think it would be worth using fused types directly for this case as well.

inplace_csr_row_normalize_l1 and inplace_csr_row_normalize_l2 are
supposed to be inplace operations, therefore they cannot by design make
a copy of the input data.

Therefore I am -1 on this part of the PR.

Agreed with the analysis. Sorry for my previous 👍, it would have been
an error to have "in-place" functions copy data.

The only correct fix for those 2 functions is to use cython fused types for
np.float32 and np.float64 (or directly cython.floating that should work for
this case AFAIK).

That would be good.

Can we close this as the incorrect patch, while waiting for a fix as part of @yenchenlin1994's GSoC?

That's what I'm doing anyway...