Skip to content

Deprecate len(..) of multi-part geometries#1075

Merged
jorisvandenbossche merged 3 commits intoshapely:masterfrom
jorisvandenbossche:deprecate-len
Jan 29, 2021
Merged

Deprecate len(..) of multi-part geometries#1075
jorisvandenbossche merged 3 commits intoshapely:masterfrom
jorisvandenbossche:deprecate-len

Conversation

@jorisvandenbossche
Copy link
Copy Markdown
Member

xref #932

PR #950 already deprecated __getitem__ and __iter__ of multi-part geometries, but not yet __len__.
Similarly to that deprecation, users can do len(obj.geoms) instead of len(obj). We could also add a .ngeoms property as a shortcut for this, if that is desired.

Note that I think deprecating __len__ is not strictly needed for compatibility with numpy (although I would need to check this to be sure), as long as the geometries are not iterable anymore (or have an array_interface). Personally I think it follows a bit from deprecating getitem/iter, where __len__ typically is implemented for collection-like objects (and which multi-part geometries no longer are in Shapely 2.0). So I am fine with deprecating it (you also need to know if you have a multi-part geometry or not before calling it, since len(point) currently fails, so if you already know that, it shouldn't be any problem to do eg len(multipoint.geoms) instead).

But, we could also keep __len__ if people prefer that. Speaking in terms of the python ABCs (https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes), multi-part geometries could be "Sized" without being "Iterable" / a "Collection".

This was referenced Jan 26, 2021
@sgillies
Copy link
Copy Markdown
Contributor

@jorisvandenbossche len(geom) used to call a GEOS method to get the number of geoms without materializing them, and was fast. As long as len(geom.geoms) is equally fast and efficient, that works for me, no need for a .ngeoms.

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

Yes, as long as we keep the .geoms attribute return a custom class and not a plain list or array, this is possible. That is also what is currently done in the shapely-2.0 branch: it kept the custom GeometrySequence class, and so checking the length of this directly calls the GEOS' method to check the number of parts of a Multi geometry:

https://github.com/Toblerity/Shapely/blob/34acbc1bd1fc12bcd07858f09133b2163fcb1e76/shapely/geometry/base.py#L705-L706

@tomplex
Copy link
Copy Markdown
Contributor

tomplex commented Jan 26, 2021

Personally I agree with your initial thought @jorisvandenbossche, len strongly implies iterability and geometries are not iterable anymore. I suspect it will be more confusing than helpful when you can ask for the len of a geometry but have to use geometry.geoms to iterate over it.

Copy link
Copy Markdown
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.

Changes look good, and I agree there is no need for a .ngeoms property.

Other than suggested edits, the manual can be adjusted slightly too:

Copy link
Copy Markdown
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.

Perfect!

@jorisvandenbossche jorisvandenbossche merged commit 47c01c0 into shapely:master Jan 29, 2021
@jorisvandenbossche jorisvandenbossche deleted the deprecate-len branch January 29, 2021 18:15
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.

4 participants