Allow scalars to be assigned to 1-element array/subarray

[BioTurboNick]

a[1:1, 2:2] = 5 currently errors because setindex_shape_check throws an error.

Changing the check to let an item through when the length of the destination is 1 allows setindex to work.

a = [1 2; 3 4]
a[1:1, 2:2] = 9
a[1:1, 1:2] = 9 # throws

Allowing this makes it easier to work with mixed arrays and scalars. Looking to see if any tests fail. At least it doesn't break anything.

[mbauman]

Hunh, I’ll have to look at this more closely. I could’ve sworn we limited iteration to AbstractArray sources.

[BioTurboNick]

Hunh, I’ll have to look at this more closely. I could’ve sworn we limited iteration to AbstractArray sources.

I guess strings are a bit weird because they define length, can be indexed, etc. Looking through Discourse it seems it's been that way for a long time, though also a common source of confusion.

Wonder if it would make sense / be possible to have a macro that opted a block out of treating strings as iterables? (I know very little about macros though.)

Base.isiterable(AbstractString) # false
Base.isiterable(String) # true

[mbauman]

Ah, ok, phew, iteration is restricted to AbstractArray RHS's (it's done in that shape check). It's documented as:

If any index I_k selects more than one location, then the right hand side X must be an array with the same shape as the result of indexing A[I_1, I_2, ..., I_n] or a vector with the same number of elements.

That should probably be written as "If any index I_k has more than 0 dimensions."

I don't think we should add a special case for a one-element (but not zero-dimensional) slice here. Indexing like A[1:1] doesn't drop back down to a scalar value. This is just the inverse — assigning to A[1:1] is assigning to an array and thus you need an array for that to work. The way to assign scalars to an array is through broadcasting, which is where we've jammed this sort of "DWIM" dimension matching.

[mbauman]

For a bit of history here, see #26347. If we're going to do this, (and I don't think we should), we need to do this more comprehensively than just whitelisting AbstractString using something like the infrastructure of _iterable (which treated all non-AbstractArrays as "scalar" — this was the behavior before #26347) or broadcastable.

[BioTurboNick]

Okay, good perspective. I'll look at those.

[mbauman]

I should also just say that the concatenation code was the worst to update because it so heavily depended upon this old behavior. So I'm not surprised you're feeling this pain and would like to to go back to the way it was. When I said we'd shoved all this sort of dimension-matching stuff into broadcast that's not entirely true. It's also in concatenation and mapslices.

[BioTurboNick]

Alright, I'm convinced by that reasoning. Thanks!