search

pysal / libpysal

Core components of Python Spatial Analysis Library
http://pysal.org/libpysal
Other
256 stars 78 forks source link

BUG: fuzzy_contiguity picks more neighbours than it should #334

Closed martinfleis closed 3 years ago

martinfleis commented 4 years ago

I am working on fuzzy_contiguity refactoring based on geopandas query_bulk and I've realized that current implementation has two bugs in it. Both in the sindex query.

https://github.com/pysal/libpysal/blob/9e6595171013296fd12777f767fbc2e7c5a1a355/libpysal/weights/util.py#L1565-L1571

1) ids = possible.intersects(geom).index.tolist() lists all indices, not only those with True value, which means that all geometries which bounds intersect with tree bounds are marked as neighbours.

2) ids.remove(i) expects RangeIndex, and raises ValueError on anything else.

This is how it looks in reality (this is actually from tests):

download

All blue polygons are marked (and tested) as neighbours, whilst the top left is clearly intersecting only bounding box.

The fix is on the way, I just have a question - what should be used as an index here (also considering https://github.com/pysal/libpysal/projects/4)? From the code, I feel the intention to use the actual index, but Queen uses integer. Due to the bug above, fuzzy_contiguity currently tries to do index, but in reality does only integer.

sjsrey commented 4 years ago

Great catch on the bugs!

On the index question you raise, I think we should use the actual index since we require a gdf in fuzzy_contiguity. Elsewhere in https://github.com/pysal/libpysal/projects/4 we have to redesign the indexing scheme anyway.