Skip to content

WIP: get parts of a geometry collection or multi* geometries#130

Closed
brendan-ward wants to merge 2 commits intopygeos:masterfrom
brendan-ward:to_parts
Closed

WIP: get parts of a geometry collection or multi* geometries#130
brendan-ward wants to merge 2 commits intopygeos:masterfrom
brendan-ward:to_parts

Conversation

@brendan-ward
Copy link
Copy Markdown
Contributor

@brendan-ward brendan-ward commented Mar 25, 2020 •
edited by jorisvandenbossche
Loading

Closes #127, closes #128

This includes a bit of refactoring to make it easier to reuse our functions and types in C.

vector: types and functions for working with resizable vectors
geom_op: arbitrary place to put operations on geometries that are not ufuncs and not at the core of creating / converting geometries

This lifted the GetGeometryN from ufuncs.c to geom_op.c for reusibility in both. There are a few other C functions in ufuncs that could move here as well (to keep the ufunc code focused on ufuncs), but weren't directly related to changes here.

We clearly need a better name for this function, get_parts is just a placeholder for now.

This takes as input a geometry or 1d array of geometry, and returns a tuple of indexes and new geometries for the parts.

GeometryCollection is not yet implemented. There are recursive aspects of that which will require a bit more refactoring here.

I did not implement the single geometry case. There might be a better way to handle that in a different function or method.

Key things to focus on here:

  • function name
  • return values

@brendan-ward brendan-ward mentioned this pull request Mar 27, 2020
Open
@caspervdw
Copy link
Copy Markdown
Member

caspervdw commented May 14, 2020

@brendan-ward I think we reached a consensus on the return value in #128

Looking at this draft PR, the work is almost done. What I think needs to be changed (functionally) is:

  • a return_indices argument that defaults to False (can be implemented on the Python level, just discard the second element)
  • Naming. the PostGIS equivalent is ST_Dump. can we name this dump?

@brendan-ward
Copy link
Copy Markdown
Contributor Author

brendan-ward commented May 14, 2020

Agreed on the use of return_indices.

Personally, I don't like ST_Dump or dump. It is ambiguous and doesn't give much information about what the function actually does. In ArcGIS and geopandas this is called explode, and it seems similar in intent to explode in pandas.

Could we go with explode or explode_parts instead?

I also need to add support for GeometryCollection via recursion, unless we want to handle that in a future PR?

@jorisvandenbossche
Copy link
Copy Markdown
Member

jorisvandenbossche commented May 14, 2020

I agree on "dump" not being a clear name IMO (I know the analogy with PostGIS is generally nice, but here I would personally not follow it)

@jorisvandenbossche
Copy link
Copy Markdown
Member

jorisvandenbossche commented May 14, 2020

Also, I personally think we should consider doing functions like PyGetParts in cython (once you need to start dealing with python function arguments, things get a lot simpler in cython), xref #51

@brendan-ward brendan-ward mentioned this pull request May 14, 2020
Closed
@brendan-ward brendan-ward mentioned this pull request Sep 24, 2020
Merged
5 tasks
@brendan-ward
Copy link
Copy Markdown
Contributor Author

brendan-ward commented Nov 3, 2020

Superseded by #197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PROPOSAL: function to return all parts of a multi-geometry as ndarray Function to "flatten" or "explode" multi-geometries

3 participants

@brendan-ward @caspervdw @jorisvandenbossche