-
Notifications
You must be signed in to change notification settings - Fork 40
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
similar with IdOffsetRanges may use the parent axes types #213
Conversation
Codecov Report
@@ Coverage Diff @@
## master #213 +/- ##
==========================================
- Coverage 98.28% 94.57% -3.71%
==========================================
Files 5 5
Lines 291 332 +41
==========================================
+ Hits 286 314 +28
- Misses 5 18 +13
Continue to review full report at Codecov.
|
Now a mix of Before: julia> s = SArray{Tuple{2,2},Int,2,4}((1,2,3,4))
2×2 SArray{Tuple{2,2},Int64,2,4} with indices SOneTo(2)×SOneTo(2):
1 3
2 4
julia> so = OffsetArray(s, 2, 2);
julia> similar(so, axes(so,1), axes(s,2))
2×2 OffsetArray(::Array{Int64,2}, 3:4, 1:2) with eltype Int64 with indices 3:4×1:2:
281474976710656 140349019967488
0 140348924690432 After 8992fc9: julia> similar(so, axes(so,1), axes(s,2))
2×2 OffsetArray(::MArray{Tuple{2,2},Int64,2,4}, 3:4, 1:2) with eltype Int64 with indices 3:4×1:2:
139 19
140348215528848 5 |
Before 8abaae2: julia> s = SArray{Tuple{2,2},Int,2,4}((1,2,3,4));
julia> so = OffsetArray(s, 2, 2);
julia> so .+ 1
2×2 OffsetArray(::Array{Int64,2}, 3:4, 3:4) with eltype Int64 with indices 3:4×3:4:
2 4
3 5 After 8abaae2: julia> so .+ 1
2×2 OffsetArray(::SizedArray{Tuple{2,2},Int64,2,2,Array{Int64,2}}, 3:4, 3:4) with eltype Int64 with indices 3:4×3:4:
2 4
3 5
julia> parent(so .+ 1) isa StaticArray
true So now broadcasting preserves the static sizes. |
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.
No objection to the fix. You always write good test cases.
src/OffsetArrays.jl
Outdated
B = similar(A, T, map(_indexlength, inds)) | ||
# strip IdOffsetRanges to extract the parent range and use it to generate the array | ||
# route through _similar to avoid a stack overflow if map(_maybeparent, inds) === inds | ||
B = _similar(A, T, map(_maybeparent, inds), inds) |
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.
It's a little bit unintuitive to understand what _similar
does, so how about:
new_inds = _strip_IdOffsetRange(inds)
B = similar(A, T, new_inds)
Currently
After this PR:
The new array type is also statically sized, as the information about the size is preserved in the axes.