-
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
Clarify ==
and ===
for axes
tests
#120
Comments
Making the tests too specific will cause them to fail when they maybe should not on changes like On the other hand, having specific tests are a way to document behavior and detect changes like that, so I'm pulled toward testing both. having an julia> @test 1:3 === OffsetArrays.IdOffsetRange(1:3)
Test Failed at none:1
Expression: 1:3 === OffsetArrays.IdOffsetRange(1:3)
Evaluated: 1:3 === 1:3
ERROR: There was an error during testing Or perhaps the definition for |
I think the actual issue is not really that we care that |
With #143 is merged, unsure whether this is good to close. julia> @test OffsetArrays.IdOffsetRange(-2:2) === OffsetArrays.IdOffsetRange(Base.OneTo(5), -3)
Test Failed at REPL[17]:1
Expression: OffsetArrays.IdOffsetRange(-2:2) === OffsetArrays.IdOffsetRange(Base.OneTo(5), -3)
Evaluated: OffsetArrays.IdOffsetRange(-2:2) === OffsetArrays.IdOffsetRange(-2:2) I guess this is only a developer-sensitive case? |
That's because those two shouldn't give the same result, but they do currently thanks to a Julia bug. After all, julia> a = OffsetArray(-4:-3, -4:-3)
-4:-3 with indices -4:-3
julia> a == -4:-3
false in just the same way that two dictionaries with equal values but different keys also don't compare as equal. JuliaLang/julia#30950 was turning into a hydra before I decided that it just needed a reboot, but I never got around to doing that. |
This is offtopic, but I'm still not clear about the difference between |
This goes back to Tim's upgrade of the axes. Essentially julia> r = Base.IdentityUnitRange(-2:2)
Base.IdentityUnitRange(-2:2)
julia> r2 = OffsetArrays.IdOffsetRange(Base.OneTo(5), -3)
OffsetArrays.IdOffsetRange(-2:2)
julia> @btime $r .+ 1;
3.732 ns (0 allocations: 0 bytes)
julia> @btime $r2 .+ 1;
3.193 ns (0 allocations: 0 bytes) |
EDIT: sorry I had not seen jishnub's reply when I wrote this. julia> pairs(OffsetArrays.IdOffsetRange(Base.OneTo(5), 2))
pairs(::OffsetArrays.IdOffsetRange{Int64,Base.OneTo{Int64}}) with 5 entries:
3 => 3
4 => 4
5 => 5
6 => 6
7 => 7
julia> pairs(Base.IdentityUnitRange(3:7))
pairs(::Base.IdentityUnitRange{UnitRange{Int64}}) with 5 entries:
3 => 3
4 => 4
5 => 5
6 => 6
7 => 7 Here is my guess: |
To be fair they are not exactly equivalent if the parent does not start at 1. julia> pairs(OffsetArrays.IdOffsetRange(5:6, 2))
pairs(::OffsetArrays.IdOffsetRange{Int64,UnitRange{Int64}}) with 2 entries:
3 => 7
4 => 8
julia> pairs(Base.IdentityUnitRange(7:8))
pairs(::Base.IdentityUnitRange{UnitRange{Int64}}) with 2 entries:
7 => 7
8 => 8 The axis of an |
In the tests I see that there are some places where
===
is used to compare results ofaxes
, which is good since1:3 == Base.OneTo(3)
, and some places want to test specifically forBase.OneTo(3)
But I also see many cases of e.g.
a == IdOffsetRange(3:5)
anda == IdentityUnitRange(-4:-3)
, and I don't see the point of doing that instead ofa == 3:5
, unless the test is really intended to check for===
, in which case in at least a few places it should bea === OffsetArrays.IdOffsetRange(Base.OneTo(3), 2)
.For example:
OffsetArrays.jl/test/runtests.jl
Line 446 in 7294f3d
The text was updated successfully, but these errors were encountered: