upgrade OffsetArrays to v1.3.0#37643
Conversation
johnnychen94
force-pushed
the
jc/upgrade_offsetarrays
branch
3 times, most recently
from
073a4df to
d702325
Compare
|
@timholy, I think this would just need a quick once-over from you. |
|
|
||
| const BASE_TEST_PATH = joinpath(Sys.BINDIR, "..", "share", "julia", "test") | ||
| isdefined(Main, :OffsetArrays) || @eval Main include(joinpath($(BASE_TEST_PATH), "testhelpers", "OffsetArrays.jl")) | ||
| isdefined(Main, :OffsetArrays) || @eval Main @everywhere include(joinpath($(BASE_TEST_PATH), "testhelpers", "OffsetArrays.jl")) |
There was a problem hiding this comment.
It surprises me that this wasn't needed before
There was a problem hiding this comment.
I think this is because eachindex(oa) requires axes(oa), which now returns OffsetArrays.IdOffsetRange.
| A | ||
| end | ||
|
|
||
| @inline function Base.deleteat!(A::OffsetArray, i::Int) |
There was a problem hiding this comment.
Interesting — is there a reason why this was removed from OffsetArrays? Or has it never made it there?
There was a problem hiding this comment.
Hmmm... I'm surprised that this seems to never appeared in OffsetArrays.jl codebase. My best guess of this not going into OffsetArrays.jl is to make clear the conceptual confusion between an array and a list.
There are tendencies to add support for OffsetList, though (e.g., JuliaArrays/OffsetArrays.jl#137, #37629)
There was a problem hiding this comment.
Makes sense — and the old definition for deleteat! doesn't really test any other base functionality, so I think this is fine to remove.
The definition for push! is also changing, though, and in doing so I think we lose the test coverage on resize!.
There was a problem hiding this comment.
The resize! tests are added in 1f9b290 (copied from OffsetArrays.jl)
v1.2.0 introduces an ambiguity and thus not used.
OffsetArrays issues 133 and 100 are fixed by JuliaLang#37204.
johnnychen94
force-pushed
the
jc/upgrade_offsetarrays
branch
from
d702325 to
c46776f
Compare
johnnychen94
changed the title
johnnychen94
force-pushed
the
jc/upgrade_offsetarrays
branch
from
c46776f to
18323b1
Compare
|
The test failure seems unrelated. |
|
Thanks @johnnychen94! |
4 participants
Separated from #37629
cc: @timholy