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: Add normalize() method #1090

Merged
merged 2 commits into from Mar 2, 2021
Merged

Conversation

jorisvandenbossche
Copy link
Member

Closes #994

This exposes GEOSNormalize as a method on the geometry classes. In GEOS, this works in place, but to keep the Shapely interface consistent (other operations like simplify also return a new geometry), I made the normalize() method to return a new geometry (and thus first clone the geometry before normalizing it).

This seems to work for me for some simple cases. cc @tomplex @Lucidiot

Especially given the overlay changes in GEOS 3.9.0 (where you often first need to normalize the geometry if you want to be able to compare it to an expected result across different GEOS versions), I think it would be useful to still include this in Shapely 1.8.

@jorisvandenbossche jorisvandenbossche added this to the 1.8 milestone Feb 15, 2021
Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

This feature should be welcome for 1.8.

shapely/geometry/base.py Outdated Show resolved Hide resolved
Co-authored-by: Mike Taves <mwtoews@gmail.com>
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 85.14% when pulling b269b39 on jorisvandenbossche:normalize into 1c56128 on Toblerity:master.

@mwtoews
Copy link
Member

mwtoews commented Mar 1, 2021

Should normalize should be a method property or regular method? E.g.:

  • Method property: line.normalize returns a geometry
  • Regular method: line.normalize() returns a geometry

I think a method property might be more fitting, since there are no arguments, and there are already many other method properties (e.g.) centroid returns a geometry.

@tomplex
Copy link
Contributor

tomplex commented Mar 2, 2021

I think centroid makes sense as a "property" because it's a geometric attribute of the geometry, whereas normalize is an operation on the geometry which is doing something to it, so a traditional method call feels right to me. Just my two cents though.

Copy link
Contributor

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

Approve! Thank you @jorisvandenbossche !

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.

Expose GEOSNormalize
5 participants