search

stevengj / GeometryPrimitives.jl

geometric primitives for piecewise functions on grids
Other
18 stars 8 forks source link

Convention for`findfirst()` return value #38

Closed smartalecH closed 1 year ago

smartalecH commented 1 year ago

In order to be more consistent with the definition of Base.findfirst(), wouldn't it be better to return the index of the shape, rather than the shape itself?

https://github.com/stevengj/GeometryPrimitives.jl/blob/7aeaa50b0b0108aa17cdc36c2209051c49181c42/src/util/kdtree.jl#L103-L111

stevengj commented 1 year ago

Yes, sounds good.

stevengj commented 1 year ago

Realize that this package was created in 2016, shortly after the release of Julia 0.5, so the findfirst API may have been different in those days.

I don't think @wsshin has been using the kd-tree code, is that right Wonseok?

wsshin commented 1 year ago

@stevengj, that's right. I don't use the k-d tree code for my FDFD stuffs, as I iterate over the vector of shapes, not over grid points.

Thanks for checking with me about potential issues before merging!