Overhaul the API

* Switch KWayMerger type to kway_merge function as API entrypoint
* Use Base's Ordering API

Author: jakobnissen

CHANGELOG.md

@@ -7,4 +7,15 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html
 ## UNRELEASED
 * Add content here that have been merged, but not made it to a release yet.
 
+## [0.2.0]
+### Breaking changes
+* The public constructor for the `KWayMerger` is now the new `kway_merge` function.
+  `KWayMerger` is public, but unexported.
+* Instead of the `F` parameter (and argument to its constructor), `kway_merge`
+  uses the same ordering API as Base's sorting functions.
+* `KWayMerger{T}` now iterates `@NamedTuple{from_iter::Int, value::T}`, to reduce
+  the risk of users conflating the two elements of the tuple.
+
+
 ## [0.1.0]
+* Initial release

src/KWayMerges.jl

@@ -1,30 +1,59 @@
 module KWayMerges
 
-export KWayMerger
+using Base.Order: Ordering, Forward, ord, lt
+
+export kway_merge
+public KWayMerger
 
 include("heap.jl")
 
 """
-    KWayMerger{T, I, F}(f::F, iterators)
-    KWayMerger{T, I}(iterators)
-    KWayMerger(f, iterators)
-    KWayMerger(iterators)
+    KWayMerger{T, I, O, S}
+
+Stateful iterator of a k-way merge of multiple iterators of the same type.
+Constructed using [`kway_merge`](@ref).
+
+The type parameters are:
+* `T`: Element type of iterators
+* `I`: Iterator type
+* `O`: Ordering, subtype of `Base.Ordering`
+* `S`: Type of state of iterators
+"""
+struct KWayMerger{T, I, O <: Base.Ordering, S}
+    ordering::O
+    iterators::Vector{I}
+    states::Vector{S}
+    heap::Vector{@NamedTuple{from_iter::Int, value::T}}
+end
+
+"""
+    kway_merge( 
+        iterators;
+        lt=isless,
+        by=identity,
+        rev::Bool=false,
+        order::Base.Order.Ordering=Base.Order.Forward
+    )
+    kway_merge(::Type{T}, ::Type{T}, iterators; kwargs...)
+    kway_merge(::Type{T}, ::Type{T}, ordering::Ordering, iterators)
 
 Create a stateful iterator which does a k-way merge between multiple
 iterators of the same type.
 
 This iterator yields `@NamedTuple{from_iter::Int, value::T}` elements, where `value` is the
 next element from one of the iterators, and `from_iter` is the 1-based index of the iterator
 that yielded `value`.
-The element `value` are chosenis from among the iterators such that, among all elements which
-are the next element of the iterators, the element is chosen which is the smallest
-according to the predicate `f::F`, which defaults to `isless`.
-
-This implies that if all iterators are sorted by `f`, the yielded will be in sorted
-order.
+The element `value` is chosen among the iterators such that, among all elements which
+are the next element of the iterators, the element is chosen which is the first
+according to the ordering.
+This implies that if all iterators are sorted by `f`, the yielded will be in sorted order.
 Hence, a `KWayMerger` is typically used to combined multiple sorted arrays
 into one sorted array.
 
+The ordering is given by the keywords `by`, `lt`, `rev` and `order` - these are the
+same as for `Base.sort!`.
+
+
 # Examples
 ```jldoctest
 julia> arrs = [[1,6], [2], [5,7], [3,4,8]];
@@ -41,31 +70,17 @@ julia> print(map(Tuple, it))
 ```
 
 # Extended help
-The type parameters are:
-* `F`: Type of function used to compare the elements. It defaults
-  to `typeof(Base.isless)`
-* `T`: Element type of iterators
-* `I`: Iterator type
-* `S`: Type of state of iterators
-
 All iterators must be of the same type. For the constructors which don't pass
 in `T` and `I` explicitly, `Base.eltype` is used
 to determine them; since its default implementation
 returns `Any`, explicitly passing them may be needed for good performance for some
 iterators.
 
 `S` is derived automatically, but this must be a fixed type;
-iterators that use states of multiple different types may
-not be supported by `KWayMerger`.
+iterators that use states of multiple different types during iteration may
+not be supported.
 """
-struct KWayMerger{T, I, F, S}
-    f::F
-    iterators::Vector{I}
-    states::Vector{S}
-    heap::Vector{@NamedTuple{from_iter::Int, value::T}}
-end
-
-function KWayMerger{T, I, F}(f::F, iterators) where {T, I, F}
+function kway_merge(::Type{T}, ::Type{I}, ordering::O, iterators) where {T, I, O}
     iters = vec(collect(iterators))
     states = nothing
     things = @NamedTuple{from_iter::Int, value::T}[]
@@ -79,25 +94,32 @@ function KWayMerger{T, I, F}(f::F, iterators) where {T, I, F}
         push!(things, (; from_iter = i, value))
         states[i] = state
     end
-    heapify!(f, things)
+    heapify!(ordering, things)
     states = if isnothing(states)
         Vector{Union{}}(undef, length(iters))
     else
         states
     end
-    return KWayMerger{T, I, F, eltype(states)}(f, iters, states, things)
+    return KWayMerger{T, I, O, eltype(states)}(ordering, iters, states, things)
 end
 
-function KWayMerger{T, I}(iterators) where {T, I}
-    return KWayMerger{T, I, typeof(isless)}(isless, iterators)
+function kway_merge(
+        ::Type{T},
+        ::Type{I},
+        iterators;
+        lt = isless,
+        by = identity,
+        rev::Bool = false,
+        order::Base.Ordering = Forward,
+    ) where {T, I}
+    ordering = ord(lt, by, rev, order)
+    return kway_merge(T, I, ordering, iterators)
 end
 
-KWayMerger(iterators) = KWayMerger(isless, iterators)
-
-function KWayMerger(f::F, iterators) where {F}
+function kway_merge(iterators; kwargs...)
     I = eltype(typeof(iterators))
     T = eltype(I)
-    return KWayMerger{T, I, F}(f, iterators)
+    return kway_merge(T, I, iterators; kwargs...)
 end
 
 # We could technically know this, but KWayMerger is stateful, and
@@ -112,11 +134,15 @@ function Base.iterate(x::KWayMerger, ::Nothing = nothing)
     state = @inbounds x.states[top.from_iter]
     it = iterate(iterator, state)
     if it === nothing
-        @inbounds heappop!(x.f, x.heap)
+        @inbounds heappop!(x.ordering, x.heap)
     else
         (new_item, new_state) = it
         @inbounds x.states[top.from_iter] = new_state
-        @inbounds heapreplace!(x.f, x.heap, (; from_iter = top.from_iter, value = new_item))
+        @inbounds heapreplace!(
+            x.ordering,
+            x.heap,
+            (; from_iter = top.from_iter, value = new_item)
+        )
     end
     return (top, nothing)
 end

src/heap.jl

@@ -1,25 +1,25 @@
-@inline function heapify!(f, xs::Vector)
+@inline function heapify!(o::Ordering, xs::Vector)
     for i in div(length(xs), 2):-1:1
-        percolate_down!(f, xs, i, xs[i])
+        percolate_down!(o, xs, i, xs[i])
     end
     return xs
 end
 
 @inline function percolate_down!(
-        f::F,
+        o::Ordering,
         xs::Vector,
         i::Integer,
         x,
-    ) where {F}
+    )
     len = length(xs)
     @inbounds while (l = 2i) <= len
         r = 2i + 1
-        j = if r > len || f(last(@inbounds(xs[l])), last(@inbounds(xs[r])))
+        j = if r > len || lt(o, last(@inbounds(xs[l])), last(@inbounds(xs[r])))
             l
         else
             r
         end
-        if f(last(@inbounds(xs[j])), last(x))
+        if lt(o, last(@inbounds(xs[j])), last(x))
             @inbounds xs[i] = xs[j]
             i = j
         else
@@ -29,20 +29,20 @@ end
     return @inbounds xs[i] = x
 end
 
-@noinline function heappop!(f, xs::Vector)
+@noinline function heappop!(o::Ordering, xs::Vector)
     isempty(xs) && throw(BoundsError(xs, 1))
     x = @inbounds xs[1]
     y = @inbounds pop!(xs)
     if !isempty(xs)
-        percolate_down!(f, xs, 1, y)
+        percolate_down!(o, xs, 1, y)
     end
     return x
 end
 
-@inline function heapreplace!(f, xs::Vector, x)
+@inline function heapreplace!(o::Ordering, xs::Vector, x)
     @boundscheck isempty(xs) && throw(BoundsError(xs, 1))
     res = @inbounds xs[1]
     @inbounds xs[1] = x
-    percolate_down!(f, xs, 1, x)
+    percolate_down!(o, xs, 1, x)
     return res
 end

test/runtests.jl

@@ -8,13 +8,13 @@ imap(f) = xs -> Iterators.map(f, xs)
 @testset "Construction" begin
     v = [1:3, 4:6, 9:11]
 
-    T = KWayMerger{Int, UnitRange{Int}, typeof(isless), Int}
+    T = KWayMerges.KWayMerger{Int, UnitRange{Int}, Base.Order.ForwardOrdering}
 
-    @test KWayMerger{Int, UnitRange{Int}, typeof(isless)}(isless, v) isa T
-    @test KWayMerger{Int, UnitRange{Int}}(v) isa T
-    @test KWayMerger(v) isa T
-    @test KWayMerger(isless, v) isa T
-    @test KWayMerger(<, v) isa KWayMerger{Int, UnitRange{Int}, typeof(<)}
+    @test kway_merge(Int, UnitRange{Int}, v; lt = isless) isa T
+    @test kway_merge(Int, UnitRange{Int}, v; rev = false) isa T
+    @test kway_merge(v) isa T
+    @test kway_merge(v; by = identity, lt = isless) isa T
+    @test kway_merge(v; lt = <) isa KWayMerges.KWayMerger{Int, UnitRange{Int}}
 end
 
 function manual_collect(it; lt = isless, by = identity)
@@ -29,15 +29,15 @@ end
 @testset "Forward sorting" begin
     v = [[3, 5, 8], [1, 1], [10, 11], [1, 2, 7], Int[]]
 
-    @test collect(KWayMerger(v)) == manual_collect(v)
+    @test collect(kway_merge(v)) == manual_collect(v)
 end
 
 @testset "Using a predicate" begin
     v = [["de", "abc"], [""], ["xysa", "dsakljdwe"]]
 
     r = manual_collect(v; by = length)
 
-    @test collect(KWayMerger((i, j) -> isless(length(i), length(j)), v)) == r
+    @test collect(kway_merge(v; rev = false, by = length)) == r
 end
 
 @testset "Reverse sorting" begin
@@ -47,16 +47,16 @@ end
     end
     r = manual_collect(v; lt = >)
 
-    @test collect(KWayMerger((i, j) -> isless(j, i), v)) == r
+    @test collect(kway_merge(v; order = Base.Order.Reverse)) == r
 end
 
 @testset "Some edge cases" begin
-    it = KWayMerger([])
-    @test it isa (KWayMerger{T, I, typeof(isless)} where {T, I})
+    it = kway_merge([])
+    @test it isa (KWayMerges.KWayMerger{T, I, Base.Order.ForwardOrdering} where {T, I})
     @test collect(it) == Any[]
 
-    it = KWayMerger([1:0, 11:10])
-    @test it isa KWayMerger{Int, UnitRange{Int}, typeof(isless)}
+    it = kway_merge([1:0, 11:10])
+    @test it isa KWayMerges.KWayMerger{Int, UnitRange{Int}, Base.Order.ForwardOrdering}
     @test collect(it) == Tuple{Int, Int}[]
     @test typeof(collect(it)) == Vector{@NamedTuple{from_iter::Int, value::Int}}
 end