[hiemstar]

Modified internal constructor of BinaryHeap:

function BinaryHeap{T,Comp}(::Comp, xs::AbstractVector{T}) where {T,Comp}
    valtree = _make_binary_heap(Comp(), T, xs)
    new{T,Comp}(Comp(), valtree)
end

to

function BinaryHeap{T,Comp}(comp::Comp, xs::AbstractVector{T}) where {T,Comp}
    valtree = _make_binary_heap(comp, T, xs)
    new{T,Comp}(comp, valtree)
end

This makes it possible to heapify an array based on data not stored in the array itself.

[codecov]

Codecov Report

Merging #564 into master will decrease coverage by 0.04%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #564      +/-   ##
==========================================
- Coverage   87.82%   87.78%   -0.05%     
==========================================
  Files          32       32              
  Lines        2087     2088       +1     
==========================================
  Hits         1833     1833              
- Misses        254      255       +1

Impacted Files	Coverage Δ
src/heaps/binary_heap.jl	97.72% <66.66%> (-2.28%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 685d33d...0a99afd. Read the comment docs.

[oxinabox]

@dillondaudert can you review?

[dillondaudert]

Hi, thanks for the PR. Could you clarify with examples what exactly this change allows?

You'll also want to make sure that all the heaps (including the MinMax heap) follow the same API and have unit tests to cover any changed functionality.

[jakobnissen]

@hiemstar It appears to me that what this PR does is allows you to instantiate a BinaryMinHeap that has the max heap property and vice versa. To me, this would break the expected behaviour of the heap. Can you clarify what you intend this PR to do?

[nsajko]

The change proposed here is merged with PR #673.