bug: GridIntersect improvements

[dbrakenhoff]

Some things I'd like to fix in gridintersect.py:

• When intersecting linestrings with GridIntersect in structured mode, and those intersections result in GeometryCollections, there are overlapping results in the intersection output. This was originally reported in bug: typo shapely function call within gridintersect #2342 (for "vertex" mode, which was fixed in fix(gridintersect): typo shapely function call within gridintersect #2342 #2343).
• Return intersection result as GeoDataFrame (instead of np.recarray) if geopandas is available.
• Remove shapely<2.0 code. It's been released for a while now so perhaps we can get rid of some old code?
• Deprecate method="structured" mode? This is somewhat complex to maintain, and performs worse than "vertex" mode in almost all cases, especially since Shapely 2.0 was released. This has the added advantage of "solving" the first issue in this list.

I'm curious to hear your thoughts about these suggestions, so let me know what you think, and I can put together a PR!

[christianlangevin]

Hi @dbrakenhoff, sounds like these are nice fixes. I think it's time to remove the shapely<2.0 code. I wonder if @mwtoews agrees? I'm also good with removing the structured mode. Doesn't seem like it's worth it to maintain. Regarding the GeoDataFrame return, should we add an optional argument to request that instead of returning based on whether or not geopandas is available? I think that could break our tests and examples. I think we will be switching more heavily to geopandas in the future, so maybe that would be a better time to make that type of change.

[mwtoews]

I think it's fine to establish a minimum shapely 2.0, as it is widely available and replaces older shapely on all supported platforms. It should make code logic much easier to review. For the record, a key improvement with shapely 2.0 is that numpy arrays of geometries are supported, and processed much more efficiently using vectorized ufuncs written in C.

[dbrakenhoff]

Alright, I'll put something together and submit it for review.

Regarding the GeoDataFrame return, should we add an optional argument to request that instead of returning based on whether or not geopandas is available? I think that could break our tests and examples. I think we will be switching more heavily to geopandas in the future, so maybe that would be a better time to make that type of change.

And good point about this, I'll make it an option and we'll see what makes sense as geopandas becomes more integrated with flopy.

[wpbonelli]

I think merging those PRs auto-closed this, sorry @dbrakenhoff

[dbrakenhoff]

No worries, I guess we leave this open until we can remove the structured method and associated code?