Fix sparse array setindex(::Int, ::Vector)#43678
Conversation
|
What I'm not sure of is what behavior should we allow, there was a short discussion here of what is allowed in the setindex of an array. But basically this is allowed, even tho it errors on broadcasting operations. julia> z = zeros(Int,4,4)
julia> z[3, :] = 10ones(2,2);
julia> z
4×4 Matrix{Int64}:
1 1 0 0
1 1 0 0
10 10 10 10
0 0 0 0 |
5b149c0 to
854f936
Compare
|
I see. That one is allowed here. As you say in the Slack thread, that (w/sh)ould be prohibited by |
|
That's the thing, when I made my branch I thought it errored and that only (1,n,1...) arrays were allowed, but apparently not. Since that is the current behaviour of normal arrays I guess sparse arrays should follow it, but IMO that should be considered a bug. In fact if it were up to me setindex should only be allowed if the shapes matched or if RHS is a vector. |
|
Shall we go with this one then @gbaraldi? The fix of the shape issue is independent and has to be made in a different place, such that this |
|
Yeah it's fine by me, it segfaulted before, so nobody will miss the weird behaviour anyway. |
|
Can you add the backport labels so it gets backported? They were present in my original PR so I imagine they should be here too. |
(cherry picked from commit dc61f29)
(cherry picked from commit dc61f29)
(cherry picked from commit dc61f29)
(cherry picked from commit dc61f29)
Alternative fix of #43644. In comparison to #43650, this avoids a double reshape via
vec-reshape, and catches the problematic case in a narrower fashion, thereby (hopefully) avoiding undesired side effects. Some little code rearrangement and removal of dead code follows in the second commit.If we go with this one, then this closes #43644 and closes #43650.