Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use more precise calculation of minimum bounding circle area #257

Merged
merged 1 commit into from Aug 2, 2023

Conversation

martinfleis
Copy link
Member

We now compute an area of a minimum bounding circle from its polygonal representation, which is not as precise as it could be. I am replacing it with shapely.minimum_bounding_radius and classic pi times the radius squared formula to avoid that.

The actual numerical difference is negligible in any useful case but there's no reason to not be more precise. And it will likely be faster this way.

@martinfleis martinfleis requested a review from ljwolf August 2, 2023 20:32
@ljwolf
Copy link
Member

ljwolf commented Aug 2, 2023

Looks good to me!

Just to characterise the numerical difference for the future:

import geopandas, geodatasets, shapely

gdf = geopandas.read_file(geodatasets.get_path("geoda ncovr"))

def reock_radius(geom):
    area = shapely.minimum_bounding_radius(geom)**2 * numpy.pi
    return shapely.area(geom) / area

def reock_polygon(geom):
    circ = shapely.minimum_bounding_circle(geom)
    return shapely.area(geom)/shapely.area(circ)

r1 = reock_radius(gdf.geometry)
r2 = reock_polygon(gdf.geometry)

from matplotlib import pyplot as plt 

f,ax = plt.subplots(1,3, figsize=(8,4))
ax[0].hist(r1, histtype='step', density=True)
ax[2].hist(r2, histtype='step', density=True, color='forestgreen')
(r1-r2).hist(ax=ax[1], density=True, bins=100)
ax[0].set_title("radius")
ax[2].set_title("polygon")
ax[1].set_title("differences")
f.tight_layout()
plt.show()

example

Using the returned polygon overstates the index consistently by .6% for US counties ((r2-r1)/r1), or about an average of .003 in magnitude.

@ljwolf
Copy link
Member

ljwolf commented Aug 2, 2023

test failures are due to adbscan, and the shape tests pass locally! I would like to merge this.

@martinfleis
Copy link
Member Author

@ljwolf Go ahead and merge!

@ljwolf ljwolf merged commit 42e9663 into pysal:main Aug 2, 2023
0 of 16 checks passed
@martinfleis martinfleis deleted the mbc branch August 3, 2023 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants