Fix undefined variables

[DanielVandH]

There are some variables/functions that are not defined in the code:

julia> report_package("SpatialIndexing"; toplevel_logger=nothing)
═════ 6 possible errors found ═════
┌ _splitseeds(node::SpatialIndexing.Node, tree::SpatialIndexing.RTree) @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\split.jl:83
│ `SpatialIndexing.SpatialIndexError` is not defined: SpatialIndexing.SpatialIndexError("Cannot split the node with less than 2 children")
└────────────────────
┌ _split!(node::SpatialIndexing.Node, tree::SpatialIndexing.RTree) @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\split.jl:248
│ `SpatialIndexing.SpatialIndexError` is not defined: SpatialIndexing.SpatialIndexError("RTree variant not supported")
└────────────────────
┌ _condense!(node::SpatialIndexing.Node, tree::SpatialIndexing.RTree, tmpdetached::AbstractVector{<:SpatialIndexing.Node}) @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\adjust.jl:39
│ no matching method found `_detach!(::typeof(SpatialIndexing.parent), ::Any, ::SpatialIndexing.RTree)`: SpatialIndexing._detach!(SpatialIndexing.parent, node_ix::Any, tree::SpatialIndexing.RTree)
└────────────────────
┌ _condense!(node::SpatialIndexing.Node, tree::SpatialIndexing.RTree, tmpdetached::AbstractVector{<:SpatialIndexing.Node}) @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\adjust.jl:45
│ `SpatialIndexing.syncmbr` is not defined: SpatialIndexing.syncmbr(SpatialIndexing.parent(node::SpatialIndexing.Node)::Any)
└────────────────────
┌ _updatembr!(node::SpatialIndexing.Branch, child_ix::Integer, child_oldmbr::SpatialIndexing.Rect, con::SpatialIndexing.SubtreeContext; force::Bool) @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\adjust.jl:87
│ no matching method found `kwcall(::@NamedTuple{force::Bool}, ::typeof(SpatialIndexing._updatembr!), ::Union{Nothing, SpatialIndexing.Branch{T, N, V}} where {T, N, V}, ::Nothing, ::SpatialIndexing.Rect, ::SpatialIndexing.SubtreeContext)` (1/2 union split): Core.kwcall(NamedTuple{(:force,)}(tuple(force::Bool)::Tuple{Bool})::@NamedTuple{force::Bool}, SpatialIndexing._updatembr!, SpatialIndexing.parent(node::SpatialIndexing.Branch)::Union{Nothing, SpatialIndexing.Branch{T, N, V}} where {T, N, V}, SpatialIndexing.pos_in_parent(node::SpatialIndexing.Branch)::Union{Nothing, Int64}, node_oldmbr::SpatialIndexing.Rect, con::SpatialIndexing.SubtreeContext)
└────────────────────
┌ delete!(tree::SpatialIndexing.RTree{T, N}, br::SpatialIndexing.Rect{T, N}) where {T, N} @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\delete.jl:9
│┌ delete!(tree::SpatialIndexing.RTree{T, N}, br::SpatialIndexing.Rect{T, N}, id::Nothing) where {T, N} @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\delete.jl:13
││┌ _condense!(node::SpatialIndexing.Leaf, tree::SpatialIndexing.RTree, tmpdetached::Vector{T1} where T1<:SpatialIndexing.Node) @ SpatialIndexing c:\Users\User\.julia\dev\SpatialIndexing.jl\src\rtree\adjust.jl:45
│││ `SpatialIndexing.syncmbr` is not defined: SpatialIndexing.syncmbr(SpatialIndexing.parent(node::SpatialIndexing.Leaf)::Union{Nothing, SpatialIndexing.Node{T, N, V}} where {T, N, V})
││└────────────────────

This addresses the issues above.

I guess the else branch in _condense! is just never reached? It would have bugged if it were ever reached (since both the if and else branches in this branch had a bug), i.e. this part

    else
        # find the entry in the parent, that points to this node
        node_ix = pos_in_parent(node)
        @assert node_ix !== nothing

        if length(node) < min_load
            # used space less than the minimum
            # 1. eliminate node entry from the parent. deleteEntry will fix the parent's MBR.
            _detach!(parent(node), node_ix, tree)
            # 2. add this node to the stack in order to reinsert its entries.
            push!(tmpdetached, node)
        else
            # global recalculation necessary since the MBR can only shrink in size,
            # due to data removal.
            tree.tight_mbrs && syncmbr!(parent(node))
        end

        return _condense!(parent(node), tree, tmpdetached)
    end

[alyst]

Thanks for finding those!
The problem is that the current tests coverage is pretty low, especially for corner cases, that's why you have errors like these.
If you have an idea about the test(s) that would expose some of these bugs, it would be nice to add to the PR.

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (9868644) 89.46% compared to head (e3e5571) 89.37%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
- Coverage   89.46%   89.37%   -0.09%     
==========================================
  Files          15       15              
  Lines         911      913       +2     
==========================================
+ Hits          815      816       +1     
- Misses         96       97       +1     

Files	Coverage Δ
src/rtree/split.jl	96.96% <0.00%> (ø)
src/rtree/adjust.jl	90.90% <0.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DanielVandH]

Usually JET.jl and Aqua.jl are best for this type of testing, although I don't know the best way to go about it right now (report_package isn't the best test, since it might miss a lot of things).

[alyst]

Do you have an idea why it's failing on nightlies?
master (which is your previous PR merged) runs fine.

[DanielVandH]

I think I changed those tests in the other PR. It's something to do with

Base.insert!(tree::RTree{T,N,SpatialElem{T,N,K,V}}, br::Rect{T,N}, id::Any, val::Any) where {T,N,K,V} =
    insert!(tree, SpatialElem{T,N,K,V}(br, id, val))

Base.insert!(tree::RTree{T,N,SpatialElem{T,N,K,V}},
             pt::Point{T,N}, id::Any, val::Any) where {T,N,K,V} =
    insert!(tree, SpatialElem{T,N,K,V}(Rect(pt), id, val))

Base.insert!(tree::RTree{T,N,SpatialElem{T,N,Nothing,V}},
             br::Union{Rect{T,N}, Point{T,N}}, val::Any) where {T,N,V} =
    insert!(tree, br, nothing, val)

where there is no longer a method error being thrown, so it keeps trying to insert and hits a nothing somewhere. I wasn't able to figure out what "cannot convert a value to nothing for assignment" meant though. Should I try and bring the MethodError back? I didn't do so previously since I didn't really understand what the purpose of those tests were.

[alyst]

| I wasn't able to figure out what "cannot convert a value to nothing for assignment" meant though. Should I try and bring the MethodError back? I didn't do so previously since I didn't really understand what the purpose of those tests were.

Ah, I think I know what's going on. Your change from MethodError to Exception was correct (looks like julia nightly changed what exception is being thrown if conversion is not possible).
So the tests on master with #13 as the head don't fail.
But this branch is based on old master without #13, so it fails, because there's still MethodError check.
Just rebase this branch on top of the current master, the CI should be fixed.

The spatial index unique leaf key has type K (typeparam). K === nothing means leaves have no keys.
There are some tests to check that you cannot provide a leaf key to a keyless tree.

[alyst]

LGTM. I'll merge it if you don't plan to add more commits.

[DanielVandH]

No more commits, good to go

[alyst]

Thanks again! I'll tag a new patch release now.