[SciPy-dev] Possible fix for scipy.sparse.lil_matrix column-slicing problem

[Robert Cimrman]

Tim Victor wrote:
> On Tue, Dec 1, 2009 at 8:11 AM, Robert Cimrman wrote:
>> I think scipy.sparse indexing should follow the behavior of numpy dense arrays.
>>  This is what current SVN scipy does (0.8.0.dev6122):
>>
>> In [1]: import scipy.sparse as spp
>> In [2]: a = spp.lil_matrix((10,10))
>> In [3]: a[range(10),0] = 1
>>
>> This is ok.
>>
>> In [5]: a[range(10),range(10)] = -1
>> In [8]: print a.todense()
>> ------> print(a.todense())
>> [[-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]]
>>
>> This is IMHO not ok (what other sparse matrix users think?)
>>
>> In [9]: import scipy as sp
>> In [10]: a[range(10),range(9,-1,-1)] = sp.rand(10)
>> In [12]: print a.todense()
 >> <snip>
>>
>> same as above...
>>
>>> I'm reminded of the Zen of Python "Explicit is better than implicit." guideline.
>> :)
>>
>> Consider also this: the current behavior (broadcasting the index arrays to the
>> whole rectangle) is not compatible with NumPy, and does not allow setting
>> elements e.g. in a random sequence of positions. On the other hand, the
>> broadcasting behaviour can be easily, explicitely, obtained by using
>> numpy.mgrid and similar functions.
>>
>>> Best regards, and many apologies for the inconvenience,
>> No problem, any help and code contribution is more than welcome!
>>
>> I guess that fixing this issue should not be too difficult, so you could make
>> another stab :-) If there is a consensus here, of course... (Nathan?)
>>
>> cheers,
>> r.
> 
> Yes, I agree with you 100%, Robert. The behavior of NumPy for dense
> arrays should be the guide, and I tried to follow it but didn't know
> to check that case.

No problem at all. It was a coincidence that I stumbled on a case that was not 
covered by the tests. I do not even uses lil_matrix much :)

> I don't defend how my version handles your case where the i and j
> indexes are both sequences. The behavior that you expect is correct
> and I plan to fix it to make your code work. I would however like to
> make sure that I understand it well and get it all correct this
> time--including correctly handling the case where the right-hand side
> is also a sequence.

Sure! I am not an expert in this either, so let's wait a bit if somebody chimes 
in... Could you summarize this discussion in a new ticket, if you have a little 
spare time?

Note that I do not push this to be fixed soon by any means, my code already 
runs ok with the current version. So take my "bugreport" easy ;)

Best,
r.