search

matplotlib / basemap

Plot on map projections (with coastlines and political boundaries) using matplotlib
MIT License
776 stars 392 forks source link

Cython fixes for compatibility with current compilers #595

Closed fweimer-rh closed 9 months ago

fweimer-rh commented 9 months ago

Both Clang 16 and (future) GCC 14 reject some invalid C due to Cython-level type errors. These changes should improve compiler compatibility.

Related to:

molinav commented 9 months ago

Thanks for this important contribution, @fweimer-rh! I think I have already been hit by these problems when building the basemap feedstock for conda-forge and MacOS, and I had to temporarily pin clang to an older version, but I had no clue about the appropriate fix.

The current GitHub workflows are passing, so everything looks smooth after your changes. Please let me have one last check in the evening (after my work), just to understand all the changes better.

molinav commented 9 months ago

I only have a question about the change of the struct names:

    ctypedef struct GEOSGeometry:
        pass
    ctypedef struct GEOSCoordSequence:
        pass

They used to be GEOSGeom and GEOSCoordSeq. Is the renaming something cosmetic, or does it come from a change in libgeos? If it comes from libgeos, does it imply that the extension can only be compiled starting with a specific GEOS version?

For example, I see that all the method-like functions still start with GEOSGeom_ or with GEOSCoordSeq_. Because of that, I would find inconsistent to rename the structs if it is only for cosmetic reasons. But I have no idea, I am not an expert on GEOS.

molinav commented 9 months ago

I had some time to review the original GEOS C headers and of course you were right with the name changes. Thanks again!