IntervalSets.jl icon indicating copy to clipboard operation
IntervalSets.jl copied to clipboard

Add `isless` and make `isequal` consistent with `hash`

Open andyferris opened this issue 7 years ago • 4 comments

The current isequal won't work in a Dict because it violates the principle that isequal(a, b) implies hash(a) == hash(b). This is corrected to not special case empty intervals (unlike == where this makes perfect sense).

I also provide an isless function so that intervals can be sorted stabily and so-on.

andyferris avatar Jul 16 '18 07:07 andyferris

The alternative of course would be to add a check for empty closed intervals in hash (and update isless accordingly). However, I think performance of Dict and sort might be slightly better with the approach here.

andyferris avatar Jul 16 '18 07:07 andyferris

This came while I was traveling without internet, sorry it didn't get attention until now.

I understand your point about Dict performance, but I kind of think that the docstring implies that isequal is more "permissive," especially about missing values, than ==. Since an empty interval is kind of like a missing value, I worry this is counter to expectations. Consequently I would favor adding the check to hash.

timholy avatar Sep 01 '18 11:09 timholy

OK. This seems consistent with how we treat empty ranges, arrays, etc.

andyferris avatar Sep 06 '18 06:09 andyferris

This is a pretty old PR, but do we still want to add the check to hash? I was also surprised by this as it was causing collisions.

julia> OpenInterval{Float64}(1, 3) == ClosedInterval{Float64}(1, 3)
false

julia> hash(OpenInterval{Float64}(1, 3)) == hash(ClosedInterval{Float64}(1, 3))
true

rofinn avatar Apr 18 '23 18:04 rofinn