-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow scalars to be assigned to 1-element array/subarray #39725
Allow scalars to be assigned to 1-element array/subarray #39725
Conversation
This comment has been minimized.
This comment has been minimized.
Hunh, I’ll have to look at this more closely. I could’ve sworn we limited iteration to |
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.)
|
Ah, ok, phew, iteration is restricted to
That should probably be written as "If any index I don't think we should add a special case for a one-element (but not zero-dimensional) slice here. Indexing like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
The key isn't that indices select more than one location, it's that they _could_ select more than one element. That is, that they have one or more dimensions. Ref: #39725
Okay, good perspective. I'll look at those. |
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 |
Alright, I'm convinced by that reasoning. Thanks! |
Ref: #39725 Co-authored-by: Nicholas Bauer <nicholasbauer@outlook.com>
Ref: JuliaLang#39725 Co-authored-by: Nicholas Bauer <nicholasbauer@outlook.com>
Ref: JuliaLang#39725 Co-authored-by: Nicholas Bauer <nicholasbauer@outlook.com>
a[1:1, 2:2] = 5
currently errors becausesetindex_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.
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.