JuliaGeometry / KDTrees.jl

KDTrees for julia
Other
25 stars 8 forks source link

InexactError for dataset with zero points #11

Closed schmrlng closed 9 years ago

schmrlng commented 9 years ago

Hi, this is a fantastic package! I noticed that this corner case, which might come up when generating these data structures programmatically, doesn't fail gracefully:

julia> KDTree(Array(Float64,3,0))
ERROR: InexactError()
 in KDTree at /Users/schmrlng/.julia/v0.3/KDTrees/src/kd_tree.jl:107
 in KDTree at /Users/schmrlng/.julia/v0.3/KDTrees/src/kd_tree.jl:101

I don't have a preference between the two fix options:

  1. The constructor checks for n_p == 0 and errors immediately
  2. The constructor does some fancier logic and returns a KDTree which will respond to every near query as empty.

so before I submit a pull request, which version do you prefer?

KristofferC commented 9 years ago

Hello, thank you for your comment.

Since there is no way of adding new points to an already created tree, I don't see any need for a tree containing zero points. Therefore I think it is best to just error out on n_p == 0.

Do you agree?

By the way, KDTree(Array(Float64,0,3)) should probably also error out.

schmrlng commented 9 years ago

I agree - I was imagining a situation where someone might be using a bunch of KDTrees to track distances for a bunch of sets, some of which may be empty. But in that case, it's probably best to let the user manually decide the behaviour they want from an empty point set, and have KDTree throw a "number of points must be positive" error at its level.

I also agree that KDTree should require the dimension to be positive.