JuliaArrays / OffsetArrays.jl

Fortran-like arrays with arbitrary, zero or negative starting indices.
Other
198 stars 40 forks source link

add non-exported helper `offset_to` #274

Closed johnnychen94 closed 2 years ago

johnnychen94 commented 2 years ago

I recently need to align all arrays into a common origin so I figure it can be helpful to put this here. I'll add docs and tests if you think it's a good idea.

codecov[bot] commented 2 years ago

Codecov Report

Merging #274 (10982a3) into master (d97c1be) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
+ Coverage   95.86%   95.88%   +0.01%     
==========================================
  Files           5        5              
  Lines         460      462       +2     
==========================================
+ Hits          441      443       +2     
  Misses         19       19              
Impacted Files Coverage Δ
src/OffsetArrays.jl 97.77% <100.00%> (+0.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d97c1be...10982a3. Read the comment docs.

jishnub commented 2 years ago

Is this really something that we need here? I feel a map is easier to read in this case.

julia> A1, A2, A3 = rand(4, 4), rand(2, 3), OffsetArray(rand(3, 2), (-1, -1)); # all have different axes

julia> A1o, A2o, A3o = map((A1, A2, A3)) do A
                           OffsetArray(A, OffsetArrays.Origin(1,2))
                       end;

julia> first.(axes(A1o))
(1, 2)

julia> first.(axes(A1o)) == first.(axes(A2o)) == first.(axes(A3o))
true
johnnychen94 commented 2 years ago

I won't say this is necessary; it's just another small helper function like the Origin and center/centered we build. Or maybe just need a better name, e.g., align_offsets.

jishnub commented 2 years ago

Maybe we can change the behavior of Origin, such that

julia> OffsetArrays.Origin(1,2)(A) == OffsetArray(A, OffsetArrays.Origin(1,2))

? In this case, we may broadcast OffsetArrays.Origin over arrays to shift their origin. The syntax seems intuitive, and this behavior of Origin is not documented, so it'll not be a breaking change either.

johnnychen94 commented 2 years ago

Looks good to me, and how about defining Origin(A::AbstractArray) as Origin(first.(axes(A)))?

jishnub commented 2 years ago

That makes sense. That way, we may transfer the origin from one array to another as Origin(A)(B).

johnnychen94 commented 2 years ago

closed in favor of #276