Make geos_to_path return one path per geometry

[rcomer]

Should this logic be moved up into the geos_to_path() function though? That currently returns a list of paths, but it seems like if you're passing in one geometry that should be a compound path like you have here.

Originally posted by @greglucas in #2325 (comment)

I have been meaning to look into this but not got around to it, so creating an issue before I forget completely!

[rcomer]

Progress so far which I did at the time of #2325 and just now rebased.

[rcomer]

I now have the tests passing in my branch 🎉 However, this function is public and a quick search of GitHub shows projects are using it. In all the examples I found, they are passing the result straight to Path.make_compound_path so this change should be an improvement for them. This does mean though that we probably need a deprecation pathway. I think to change this function in place we would need two deprecations

1. Introduce a parameter to opt in to new behaviour, deprecate not using that parameter
2. Make the new behaviour default and deprecate the parameter

This seems unappealing with a fair bit of code churn for us and users. Am I missing a better way?

Another option might be to deprecate this function entirely and introduce a new function for the new behaviour. Name suggestions: geom_to_path or maybe shapely_to_path, since it handles shapely collections as well as individual geometries.

[greglucas]

🎉 Nice!

I almost think we could consider this a bugfix to a certain extent... I don't think we want to deal with a double deprecation like you're saying that would be painful. I'd vote for either ripping the bandaid off and calling it a bugfix, or switching to a new function with a deprecation attached to the current one.

[rcomer]

A disadvantage of making a new function is that we would lose the naming symmetry with path_to_geos 🤔

[greglucas]

Add a path_to_shapely function as well? I'm also curious if that needs to be updated for the compound path as input that you are working on here too. Is there even an exact one to one mapping we can guarantee, or are there some cases that will cause issues going one direction or the other in a non-unique way?

[rcomer]

Good point. path_to_geos also always returns a list. The list may contain

• one or more Polygons (which were closed paths) or
• a single Linestring or MultiLinestring (which were open paths) or
• a mixture of polygons and linestrings

So maybe path_to_shapely should return a single shapely object which could be a MultiPolygon or a GeometryCollection. project_geometry does not currently support GeometryCollections, but I assume we can just do that with a list comprehension.

Aside: when I see "geos" I tend to think of https://libgeos.org/, so quite like a name change from that point of view too.

[greglucas]

We used to wrap libgeos directly which made more sense for that name, but now rely on shapely to handle that interface for us. I'm actually curious if shapely even has some of these functions already for us since they do plot their geometries too... this seems like it would be more useful than just Cartopy to me with plotting collections of geometries as a compound path in matplotlib in an efficient manner. I feel like I've seen an issue in matplotlib discussing a shapely plot routine as well.

[rcomer]

Yeah, I had a look at their plotting module (which is marked experimental). AFAICS they don't have anything that returns the path here - just makes a PathPatch and plots it.
https://github.com/shapely/shapely/blob/main/shapely/plotting.py