Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests for gdal 3.4 and 3.6 #352

Merged
merged 28 commits into from
Feb 26, 2024
Merged

Conversation

margrietpalm
Copy link
Contributor

@margrietpalm margrietpalm commented Feb 15, 2024

Fix gdal 3.6 issues

  • Add EPSILON (=1e-4) to corners of area to refine used in test_rasterize_poly_refinement and test_rasterize_multiple_refinements to fixtures and return based on gdal version
  • Add tests with gdal 3.6 to github actions (for python 3.12) using a ppa to get gdal 3.6

Cases where the area to refine exactly matches the computational grid may have different refinements in gdal 3.4 then in gdal 3.6. Users should be informed about this on release

Fix gdal 3.4 issues

Allow failing (skipped) test ./threedigrid_builder/tests/test_raster_interfaces.py::test_crs to pass by splitting the tests and running one or
the other based on the gdal version. This however does not ensure that there are no downstream effects.

Given

current = CRS(CRS.from_epsg(28992).to_wkt())
legacy =  CRS.from_wkt(
        'PROJCS["Amersfoort / RD New",GEOGCS["Amersfoort",DATUM["Amersfoort",'
        'SPHEROID["Bessel 1841",6377397.155,299.1528128,AUTHORITY["EPSG","7004"]],'
        "TOWGS84[565.2369,50.0087,465.658,-0.406857,0.350733,-1.87035,4.0812],"
        'AUTHORITY["EPSG","6289"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],'
        'UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4289"]],'
        'PROJECTION["Oblique_Stereographic"],PARAMETER["latitude_of_origin",52.1561605555556],'
        'PARAMETER["central_meridian",5.38763888888889],PARAMETER["scale_factor",0.9999079],'
        'PARAMETER["false_easting",155000],PARAMETER["false_northing",463000],'
        'UNIT["metre",1,AUTHORITY["EPSG","9001"]],AXIS["Easting",EAST],AXIS["Northing",NORTH],'
        'AUTHORITY["EPSG","28992"]]'
    )

There is one call that gives a different result:

> current.to_epsg()
28992
> legacy.to_epsg
None

This call is used the property RasterIORasterInterface.epsg_code, which is subsequently used by RasterHasMatchingEPSGCheck.is_valid where it could potentially change the result. This function seemed to be well tested, so any issues here should be caught.

Furthermore, returning the actual spsg instead of None seems an improvement.

Partially updated github action matrix

@margrietpalm margrietpalm changed the title WIP: fix gdal 3.4 fix tests for gdal 3.4 Feb 15, 2024
@margrietpalm margrietpalm changed the title fix tests for gdal 3.4 WIP: fix tests for gdal 3.4 Feb 15, 2024
@margrietpalm margrietpalm changed the title WIP: fix tests for gdal 3.4 fix tests for gdal 3.4 Feb 15, 2024
@margrietpalm margrietpalm changed the title fix tests for gdal 3.4 fix tests for gdal 3.4 and 3.6 Feb 16, 2024
@margrietpalm margrietpalm changed the title fix tests for gdal 3.4 and 3.6 WIP: fix tests for gdal 3.4 and 3.6 Feb 16, 2024
@margrietpalm margrietpalm changed the title WIP: fix tests for gdal 3.4 and 3.6 fix tests for gdal 3.4 and 3.6 Feb 20, 2024
@margrietpalm margrietpalm changed the title fix tests for gdal 3.4 and 3.6 WIP: fix tests for gdal 3.4 and 3.6 Feb 20, 2024
@margrietpalm margrietpalm changed the title WIP: fix tests for gdal 3.4 and 3.6 Fix tests for gdal 3.4 and 3.6 Feb 21, 2024
@margrietpalm margrietpalm requested review from daanvaningen and removed request for martijn-siemerink February 22, 2024 09:00
Copy link
Contributor

@daanvaningen daanvaningen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎖️

@margrietpalm margrietpalm merged commit ba53d94 into master Feb 26, 2024
8 checks passed
@margrietpalm margrietpalm deleted the margriet_257_fix_gdal_3.4 branch February 26, 2024 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants