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

ENH: support single-part multipolygons in Squareness and CentroidCorners #507

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

songololo
Copy link
Contributor

Is there appetite to support MultiPolygons for Squareness and CentroidCorners?

@martinfleis
Copy link
Member

thanks for this!

I understand how useful it could be in some cases but am not sure if it doesn't bring a bit opaque inconsistency when it does work for some MultiPolygons but not for the other.

@jGaboardi what are your thoughts?

@jGaboardi
Copy link
Member

I understand how useful it could be in some cases but am not sure if it doesn't bring a bit opaque inconsistency when it does work for some MultiPolygons but not for the other.

It seems to me that the opaque inconsistency can be cleared up with several more words in the docstring (and maybe a thrown warning silenced by default at the absolute most). My reasoning here is that the MultiPolygons that are being operated on are not true MultiPolygons, they are single Polygons mislabeled (perhaps not the best terminology) as being MultiPolygons. Maybe something like:

Returns ``np.nan`` for true MultiPolygons (containing multiple geometries).
MultiPolygons with a singular geometry are treated as Polygons.

And tests failing due to #508, not this PR, correct?

@jGaboardi jGaboardi added the enhancement New feature or request label Aug 21, 2023
@martinfleis
Copy link
Member

Okay, let's give it a shot following @jGaboardi's suggestion.

And tests failing due to #508, not this PR, correct?

Yes.

momepy/shape.py Outdated Show resolved Hide resolved
momepy/shape.py Outdated Show resolved Hide resolved
momepy/shape.py Outdated Show resolved Hide resolved
momepy/shape.py Outdated Show resolved Hide resolved
songololo and others added 3 commits August 21, 2023 17:05
Adds more explanation regarding application to MultiPolygons

Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Adds more explanatory text on application to MultiPolygons

Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
@martinfleis martinfleis changed the title adds multipolygon support for Squareness and CentroidCorners ENH: support single-part multipolygons in Squareness and CentroidCorners Aug 21, 2023
Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@martinfleis martinfleis merged commit 4f1176a into pysal:main Aug 21, 2023
4 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants