Toblerity / Fiona

Fiona reads and writes geographic data files
https://fiona.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
1.14k stars 202 forks source link

LINESTRING M's are converted to LINESTRING Z's. Please fix Windows Wheels by bumping GEOS to 3.12. #1309

Closed sentientnebula closed 10 months ago

sentientnebula commented 10 months ago

I was excited when I saw Fiona 1.9.5 posted because I just assumed the next wheels would be built with GEOS 3.12 which finally fixed the issues with reading/writing Linestring M's and ZM's. I was surprised to see it bumped to 3.11.2 instead of 3.12 since Fiona 1.9.5 was released post GEOS 3.12 release.

I know GEOS still doesn't actually support doing much with the M values except reading/writing them correctly, but that is fine. I just strip off the M's when manipulating them, calculate new measurements when I am done, and form them into WKT LINESTRING M's which I then need to wkt.loads into a dataframe to export to a geopackage.

I am pretty sure an update to GEOS 3.12 is all that is needed to allow me to do this.

sgillies commented 10 months ago

@sentientnebula thank you for the suggestion! Can you please link to the GEOS change log entry or issue that discusses the problem and fix so I can learn more about this?

I'm not opposed to making a 1.9.5.post1 release to do this. A few weeks ago I would have been blocked, as vcpkg (which we use for Windows) is still on GEOS 3.11.2: https://github.com/microsoft/vcpkg/commits/master/versions/g-/geos.json. Recently, I've figured out how to work around that, and can probably make a local override to use GEOS 3.12 if the GEOS build system hasn't changed.

sentientnebula commented 10 months ago

From 3.12 announcement on GEOS Website :

GEOS has been updated to read and write “M” dimension. As with the “Z” coordinate support, not all operations can preserve the “M” dimension, but best efforts are made to retain it through calculations, input and output.

https://libgeos.org/posts/2023-06-27-geos-3-12-released/

Github commits:

https://github.com/libgeos/geos/pull/721/commits

sgillies commented 10 months ago

@sentientnebula I've thought about this a bit more and I wonder if there may be some confusion about the features of the fiona and shapely packages. wkt.loads is from shapely, not fiona. And fiona does not call any GEOS methods at all, only GDAL/OGR methods. See for example https://github.com/Toblerity/Fiona/blob/master/fiona/_geometry.pyx#L277.

mwtoews commented 10 months ago

Related on the shapely side, I'm planning to implement some M and ZM support for upcoming shapely-2.1.

Are you sure GDAL/OGR needs to be built with GEOS 3.12? I thought M/ZM support for GDAL/OGR was stand-alone, but correct me if needed. On the Fiona-side, more work is needed to implement, e.g., OGR_G_AddPointM and other higher-dimension CAPI methods.

sentientnebula commented 10 months ago

@sgillies That is why I said I don't think there are any changes you need to make to Fiona to get it to work other than updating GEOS so that GDAL/OGR can call the fixed GEOS that no longer returns M's as Z's. It should make Shapely's wkt.loads just start working because GDAL/OGR functions should start returning correct linestrings to Fiona when GEOS returns the correct linestrings to GDAL/OGR.

@mwtoews As far as I know, there is no way to get GDAL/OGR to support M/ZM without being built with GEOS 3.12. I realize that work will need to be done to get Shapely to actually manipulate M values, but I wouldn't expect you to need to do anything to make wkt.loads work other than get the corrected strings back from GDAL/OGR when it gets the correct ones from GEOS. If that is not the case, for some reason, I can just call the GDAL/OGR WKT conversion fuctions directly.

sgillies commented 10 months ago

@mwtoews Right, I'm pretty sure that Fiona's serialization does not involve GEOS. I'll make a note about Z and M support there.

mwtoews commented 10 months ago

The API docs specify which functions require GEOS (e.g. OGR_G_Simplify).

GDAL/OGR has had M support since 2.1, see rfc 61.

rouault commented 10 months ago

GDAL supported M since 2.1 indeed, but when exporting geometries to GEOS, it dropped the M component. https://github.com/OSGeo/gdal/pull/8652 that will hopefully make it in GDAL 3.8.0 no longer does that with GEOS >= 3.12

sentientnebula commented 10 months ago

see last comment by jorisvandenbossche :

https://github.com/shapely/shapely/issues/882

He got Shapely's wkt.loads function to work properly just by updating GEOS to 3.12, which for most Windows users would come via an update to the GEOS package bundled with Fiona.

I'm not using Windows by choice. I use Linux on all my home machines, where this wouldn't be an issue, but building my own wheels for packages with C++ extensions on a Windows work machine with user rights very heavily locked down by our IT department is something I'd rather not attempt.

sgillies commented 10 months ago

He got Shapely's wkt.loads function to work properly just by updating GEOS to 3.12, which for most Windows users would come via an update to the GEOS package bundled with Fiona.

@sentientnebula GeoPandas depends on both shapely and fiona, but shapely and fiona are independent packages. Fiona does not upgrade shapely, at least not by design or specification. I'm going to close this issue. Better support for M and Z needs work in fiona's _geometry module and GEOS 3.12 won't help.