search

geocaml / ocaml-rtree

A pure OCaml implementation of R-Trees
BSD 3-Clause "New" or "Revised" License
26 stars 7 forks source link

Remove fns #38

Open FayCarsons opened 5 months ago

FayCarsons commented 5 months ago

Implemented removal by equality and by envelope in a way I'm pretty happy about.

Testing is needed (which, I'm not super sure how to go about that) and I think the aux functions for both can be combined into a single removal by predicate fn? But there's also potential branching optimizations in the case of the removal by envelope operation that may clash with that? Will investigate!

patricoferris commented 4 months ago

Sorry for the delay!

FayCarsons commented 4 months ago

Thanks for the feedback! Applied suggestions and added unit test, will try to add more later when I can take a closer look at the test suite.

FayCarsons commented 4 months ago

Added more unit tests. Testing removal on an empty tree, removal from a tree w/ one elt, removing an elt that doesn't exist, removing a lot of elts. I wanted to add a test for removing from a degenerate tree but wasn't sure how to construct a degenerate tree. Can explore later. Fixed doc comments.

FayCarsons commented 4 months ago

Added another unit test that inserts 100 random elts, half within an envelope and half out, and then removes the half within the envelope.

I'm not super experienced with unit testing, would love any suggestions for new unit tests if this is not comprehensive enough!