Wrong IteratorSize for pairs(::IndexCartesian, ...)

[martinholters]

On Julia master:

julia> Base.IteratorSize(pairs(IndexCartesian(), zeros(2,2)))
Base.HasShape{2}()

On Julia 0.6 with Compat:

julia> Compat.IteratorSize(Compat.pairs(IndexCartesian(), zeros(2,2)))
Base.HasLength()

We implicitly test for the latter behavior, making our tests fail on 0.7. We probably want to do

diff --git a/test/runtests.jl b/test/runtests.jl
index 4201f5fe..05bbf174 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -778,9 +778,9 @@ let
     A14 = [11 13; 12 14]
     R = CartesianIndices(Compat.axes(A14))
     @test [a for (a,b) in pairs(IndexLinear(),    A14)] == [1,2,3,4]
-    @test [a for (a,b) in pairs(IndexCartesian(), A14)] == vec(collect(R))
+    @test [a for (a,b) in pairs(IndexCartesian(), A14)] == collect(R)
     @test [b for (a,b) in pairs(IndexLinear(),    A14)] == [11,12,13,14]
-    @test [b for (a,b) in pairs(IndexCartesian(), A14)] == [11,12,13,14]
+    @test [b for (a,b) in pairs(IndexCartesian(), A14)] == [11 13; 12 14]
 end
 
 # Val(x)

But that requires fixing the result of pairs, which currently relies on IndexValue on 0.6. Should we replace the whole implementation

Compat.jl/src/Compat.jl

Lines 574 to 614 in 9ae1261

	pairs(collection) = Base.Generator(=>, keys(collection), values(collection))
	pairs(a::Associative) = a
	# 0.6.0-dev+2834
	@static if !isdefined(Iterators, :IndexValue)
	include_string(@__MODULE__, """
	immutable IndexValue{I,A<:AbstractArray}
	data::A
	itr::I
	end
	""")
	Base.length(v::IndexValue) = length(v.itr)
	Base.indices(v::IndexValue) = indices(v.itr)
	Base.size(v::IndexValue) = size(v.itr)
	@inline Base.start(v::IndexValue) = start(v.itr)
	Base.@propagate_inbounds function Base.next(v::IndexValue, state)
	indx, n = next(v.itr, state)
	item = v.data[indx]
	(indx => item), n
	end
	@inline Base.done(v::IndexValue, state) = done(v.itr, state)
	Base.eltype{I,A}(::Type{IndexValue{I,A}}) = Pair{eltype(I), eltype(A)}
	Base.iteratorsize{I}(::Type{IndexValue{I}}) = iteratorsize(I)
	Base.iteratoreltype{I}(::Type{IndexValue{I}}) = iteratoreltype(I)
	Base.reverse(v::IndexValue) = IndexValue(v.data, reverse(v.itr))
	else
	const IndexValue = Iterators.IndexValue
	end
	pairs(::IndexLinear, A::AbstractArray) = IndexValue(A, linearindices(A))
	pairs(::IndexCartesian, A::AbstractArray) = IndexValue(A, CartesianRange(indices(A)))
	Base.keys(a::AbstractArray) = CartesianRange(indices(a))
	Base.keys(a::AbstractVector) = linearindices(a)
	Base.keys(s::IndexStyle, A::AbstractArray, B::AbstractArray...) = eachindex(s, A, B...)
	Base.values(itr) = itr

with the current one from Base?

[nalimilan]

I'm afraid we cannot change the existing implementation on 0.6 or it could break packages that rely on it. So maybe just make the test conditional on the Julia version? Anyway it's a relatively obscure method.

BTW, JuliaLang/julia#27038 will change this test again.

[martinholters]

I really don't know enough about the details to make a call on how to proceed here, but...

So maybe just make the test conditional on the Julia version?

That sounds like the opposite of what Compat should do. If the best we can do is that pairs(IndexCartesian(), ...) has slightly different behavior depending on Julia version, then I guess we should make the test independent of the difference and add a brief warning to the README.

[nalimilan]

In practice it would be equivalent whether we test specific behaviors on each version, or whether we call vec before checking the result on all versions. I guess we can add a warning but that's really not a common method so not a big deal.

[martinholters]

So just add vec on both sides of the == and live with the slight difference across Julia versions? Would be OK with me.

[martinholters]

Closing as outdated since we've dropped support for Julia prior to 1.0 and removed the code in question.