Get rid of special-casing of ranges in == and isequal() for AbstractArrays

[nalimilan]

The AbstractArray default == operator special-cases ranges:

function (==)(A::AbstractArray, B::AbstractArray)
    if size(A) != size(B)
        return false
    end
    if isa(A,Range) != isa(B,Range)
        return false
    end
    for (a, b) in zip(A, B)
        if !(a == b)
            return false
        end
    end
    return true
end

isequal is defined in the same way.

This is obviously not great. A new array type is considered as equal to a standard Array if their elements are all equal, but it is considered as different from a range containing the same elements. This inconsistency is visible even inside Base:

julia> sparse([1,2]) == [1,2]
true

julia> [1,2] == 1:2
false

So it's not clear whether two AbstractArrays need to be of the same type or not to be ==. I can see two consistent solutions:

1. remove the special-case for ranges (i.e. return true when elements are equal)
2. change if isa(A,Range) != isa(B,Range) to typeof(A) !== typeof(B)

The second choice would clearly be much more disruptive than the first, and it would make == almost useless for arrays.

[timholy]

#13565

[nalimilan]

Ah, funny how I keep bumping into this issue without remembering... :-)

Anyway, should we reopen one of the older issues about hashing ranges? This looks like a significant inconsistency in the language to me (and equality is already complex to master without it...).

[timholy]

The concern is that it's an unsolvable problem, in which case reopening the issue won't help.

[nalimilan]

Well, a few ideas were advanced here: #12226 (comment)

It would be really sad that == wouldn't give the expected result just to ensure fast hashing of ranges. It seems to me that == is a much more common operation on ranges than hash (let alone hashing float ranges...).

[timholy]

👍 Skip the discussion, go straight for the PR, then 😄. EDIT: meaning, fix the hashing algorithm so you get the best of all worlds.

[nalimilan]

I didn't expect I would be able to do something about it, but it might be easier than I/we thought. Please comment on #16401.

[StefanKarpinski]

I rather suspect this is not going to happen in 0.6.

[StefanKarpinski]

Note that this is also not breaking – it's technically an optimization.

[nalimilan]

Note that this is also not breaking – it's technically an optimization.

You mean that we should make ranges and arrays hash equal by iterating over all of their elements, and try to optimize this later?

[StefanKarpinski]

Ah yes, I thought that was what we were already doing, actually!

[StefanKarpinski]

I kind of suspect that hashing ranges is not all that common, tbh.

[nalimilan]

Yes, that's also my opinion. == is much more useful.

[mbauman]

So should we squeeze the breaking portion of this into 0.6? It's a very easy change to make — just delete some code and fix a few tests. But changing the algorithmic complexity of hash from O(1) to O(n) could be terribly breaking… if anyone happens to be hashing large ranges their program suddenly becomes (effectively) non-terminating. hash(1:typemax(Int64)) goes from taking under a microsecond to over a hundred millennia.

Or we could alternatively add a depwarn for hash of relatively large ranges.

[StefanKarpinski]

I'd vote for leaving this alone until we have a real plan for this.