index.delete requires unique ids and errors not thrown

[thehesiod]

per http://toblerity.org/rtree/class.html#rtree.index.Index.delete it states for the id parameter to delete: long integer A long integer that is the identifier for this index entry. IDs need not be unique to be inserted into the index, and it is up to the user to ensure they are unique if this is a requirement.

However I tried this with with an Index which had a set of points which all had the same ids and it never removed it items. As soon as I gave each point a unique id the points got removed.

There are two bugs here:

1. documentation should be updated or implementation fixed
2. the python code should be checking error code returned by Index_DeleteData and throwing exception if the delete failed. Probably should go over the whole index.py file to make sure returned error codes are enforced.

[sr-murthy]

@thehesiod I assume this remains an open issue - if so can I work on it?

[hobu]

This is still an open issue, but I think it is a documentation one. libspatialindex indexes by bounding boxes + id, and to delete a unique record, you need to provide both.

[sr-murthy]

Yes, this makes sense because insertion of entries requires a (not necessarily unique) ID and a bounding box. It does seem strange that insertion of duplicate entries (entries with identical IDs and bounding boxes) does not throw an error - should this in fact be addressed first before the delete issue?

In [1]: from rtree.index import Index

In [2]: idx = Index()

In [3]: idx.insert(1, (0.0, 0.0, 0.0, 0.0), obj=1)                                                                               

In [4]: idx.insert(1, (0.0, 0.0, 0.0, 0.0), obj=1)                                                                               

In [5]: idx.leaves()                                                                                                             
Out[5]: [(0, [1, 1], [0.0, 0.0, 0.0, 0.0])] 

Why is this allowed in the first place?

[hobu]

libspatialindex allows it. It is up to user applications to prevent it from happening.

[thehesiod]

If it's a doc problem that is fine, let's get it fixed :)

[sr-murthy]

OK, I'm happy to work on it.

[sr-murthy]

I've created a PR here

#133

that clarifies the docstring for index.Index.delete. I hope it addresses the documentation side of this issue. Clearly, as the index allows insertion of multiple items/entries with identical IDs and coordinates, it is up to the user to enforce uniqueness at the application level.

[sr-murthy]

@hobu @thehesiod What should be done about the error/exception issue?

[thehesiod]

That's a question for hobu

[hobu]

I think documentation is sufficient. I don't think Rtree should throw an exception in this instance. It is possible users have a reason to insert duplicates, and they should protect against that themselves if they need to do so.