JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.61k stars 5.48k forks source link

union(a,b) return type should be the same as `union(b,a)` #29481

Open dlfivefifty opened 6 years ago

dlfivefifty commented 6 years ago

This is surprising:

julia> union([1,2,3], Set([4,5,6]))
6-element Array{Int64,1}:
 1
 2
 3
 4
 5
 6

julia> union(Set([4,5,6]), [1,2,3])
Set([4, 2, 3, 5, 6, 1])

In the process of fixing this, I'd propose support more general union types (like Intervals, see https://github.com/JuliaMath/IntervalSets.jl/issues/41) I'd propose redesigning union to use a lazy Unioned intermediary that mimics Broadcasted. I might make a proof-of-concept in LazyArrays.jl.

CC @timholy

oscardssmith commented 6 years ago

Union and intersection should probably always return sets. It's almost always the most efficient method, and it would improve consistency.

StefanKarpinski commented 6 years ago

For better or for worse, this is pretty much written in stone until 2.0.

dlfivefifty commented 6 years ago

For better or for worse, this is pretty much written in stone until 2.0.

Sure, this can wait til 2.0. In the meantime I'll do a proposed "replacement" in LazyArrays.jl that can be considered for moving into Base when 2.0 development starts.

Union and intersection should probably always return sets

I assume you mean a subtype of AbstractSet: if we have sparse vectors a Set won't necessarily be the most efficient. I'm also not sure this works when its infinite sets like intervals, where the elements cannot be enumerated, but that depends on the currently undefined definition of AbstractSet.

oscardssmith commented 6 years ago

Yeah, that's probably true.

matbesancon commented 6 years ago

at least it can be specified in the docstrings, something like union(S{A},I{B}) -> S{C} where elements of both types A and B are promotable/convertible to C

dlfivefifty commented 6 years ago

Here's a proposed solution: https://github.com/JuliaArrays/LazyArrays.jl/blob/master/src/lazysetoperations.jl