Skip to content
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

reshape: helpful error message, more tests #15973

Merged
merged 3 commits into from
Apr 21, 2016
Merged

reshape: helpful error message, more tests #15973

merged 3 commits into from
Apr 21, 2016

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Apr 20, 2016

I was intending to add a NEWS item (since added by Jeff), but discovered a few more loose ends. This adds an error message:

 julia> a = reshape(1:12, 4, 3)
4×3 Base.ReshapedArray{Int64,2,UnitRange{Int64},Tuple{}}:
 1  5   9
 2  6  10
 3  7  11
 4  8  12

julia> a[3,2] = -1
ERROR: indexed assignment fails for a reshaped range; consider calling collect
 in setindex!(::Base.ReshapedArray{Int64,2,UnitRange{Int64},Tuple{}}, ::Int64, ::Int64, ::Int64) at ./reshapedarray.jl:138
 in eval(::Module, ::Any) at ./boot.jl:236

This also tests more explicitly for the sharing behavior than before.

If the bitarray test runs before core, it defines a variable m1 that causes trouble for core
@timholy timholy merged commit 9b029a3 into master Apr 21, 2016
@timholy timholy deleted the teh/more_reshape branch April 21, 2016 17:48
a = reshape(1:20, 5, 4)
for idx in ((3,), (2,2), (Base.ReshapedIndex(1),))
try
a[idx...] = 7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is supposed to always fail, needs an error("unexpected") or something so it doesn't silently pass

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants