Yes I was. 

Maybe I am paranoid but each time I see an in place modification I check that it does not apply to input arguments. Thus I think that l might see the potential problem also in a later code review. 

I think that it is good practice not to let such a code.  If m is an intégré there is no computational benefit to doing the +=.

<div>-------- Original message --------</div><div>From: Lars Buitinck <notifications@github.com> </div><div>Date:29/04/2014  23:37  (GMT+01:00) </div><div>To: scikit-learn/scikit-learn <scikit-learn@noreply.github.com> </div><div>Cc: Gael Varoquaux <gael.varoquaux@normalesup.org> </div><div>Subject: Re: [scikit-learn] [MRG+1] ENH: Swap rows in sparsefuncs (#3104) </div><div>
</div>In sklearn/utils/sparsefuncs.py:

> @@ -56,3 +57,109 @@ def inplace_column_scale(X, scale):
>      else:
>          raise TypeError(
>                  "Unsupported type; expected a CSR or CSC sparse matrix.")
> +
> +
> +def swap_row_csc(X, m, n):
> +    """
> +    Swaps two rows of a CSC matrix in-place.
> +
> +    Parameters
> +    ----------
> +    X : scipy.sparse.csc_matrix, shape=(n_samples, n_features)
> +    m : int, index of first sample
> +    n : int, index of second sample
> +    """
> +    if m < 0:
> +        m += X.shape[0]
I don't get that:

>>> x = y = np.int(0)
>>> y += 1
>>> x, y
(0, 1)
Gael, where you thinking of 0-d arrays?

>>> x = y = array(0)
>>> x += 1
>>> x, y
(array(1), array(1))
I'm not sure if guarding for this is worthwhile. It's very likely that a later refactoring round undoes this; callers should take care not to produce these values.

—
Reply to this email directly or view it on GitHub.