yeesian / ArchGDAL.jl

A high level API for GDAL - Geospatial Data Abstraction Library
https://yeesian.github.io/ArchGDAL.jl/stable/
Other
141 stars 27 forks source link

Fix of custom GDAL error handler management by deregistrating it at exit (done in `__init__()`). Plus a few modifications on GDAL utilities' tests and #245

Closed mathieu17g closed 2 years ago

mathieu17g commented 3 years ago

Fixes #241 and some other issues in GDAL utilities tests

mathieu17g commented 3 years ago

Maybe the issue turns around catching GDAL.GDALError ? Here is below a test with and without catching the error and and test with a valid option. All three followed by an exit()` -> No link with catching the error

julia> cd("/Users/Mathieu/.julia/dev/ArchGDAL/test/")

julia> AG.read("data/utmsmall.tif") do ds_small try AG.gdalinfo( ds_small, ["-novalidoption"], ) catch e string(e) end end "GDAL.GDALError(GDAL.CE_Failure, 6, \"Unknown option name '-novalidoption'\")"

julia> exit() CPLDestroyMutex: Error = 16 (Resource busy)

[Opération terminée]

- Without catching the error:
```julia
julia> using ArchGDAL; const AG = ArchGDAL
ArchGDAL

julia> cd("/Users/Mathieu/.julia/dev/ArchGDAL/test/")

julia> AG.read("data/utmsmall.tif") do ds_small
           AG.gdalinfo(ds_small, ["-novalidoption"])
       end
ERROR: GDALError (CE_Failure, code 6):
    Unknown option name '-novalidoption'

Stacktrace:
 [1] gdaljl_errorhandler(class::GDAL.CPLErr, errno::Int32, errmsg::Cstring)
   @ GDAL ~/.julia/packages/GDAL/ycHfN/src/error.jl:36
 [2] gdalinfooptionsnew(papszArgv::Vector{String}, psOptionsForBinary::Ptr{Nothing})
   @ GDAL ~/.julia/packages/GDAL/ycHfN/src/GDAL.jl:18417
 [3] gdalinfo(dataset::ArchGDAL.Dataset, options::Vector{String})
   @ ArchGDAL ~/.julia/packages/ArchGDAL/zkx2f/src/utilities.jl:15
 [4] #1
   @ ./REPL[3]:2 [inlined]
 [5] read(f::var"#1#2", args::String; kwargs::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ ArchGDAL ~/.julia/packages/ArchGDAL/zkx2f/src/context.jl:267
 [6] read(f::Function, args::String)
   @ ArchGDAL ~/.julia/packages/ArchGDAL/zkx2f/src/context.jl:265
 [7] top-level scope
   @ REPL[3]:1

julia> exit()
CPLDestroyMutex: Error = 16 (Resource busy)

[Opération terminée]

-> No link with catching the error

julia> cd("/Users/Mathieu/.julia/dev/ArchGDAL/test/")

julia> AG.read("data/utmsmall.tif") do ds_small AG.gdalinfo(ds_small, ["-stats"]) end "Driver: GTiff/GeoTIFF\nFiles: data/utmsmall.tif\nSize is 100, 100\nCoordinate System is:\nPROJCRS[\"NAD27 / UTM zone 11N\",\n BASEGEOGCRS[\"NAD27\",\n DATUM[\"North American Datum 1927\",\n ELLIPSOID[\"Clarke 1866\",6378206.4,294.978698213898,\n LENGTHUNIT[\"metre\",1]]],\n PRIMEM[\"Greenwich\",0,\n ANGLEUNIT[\"degree\",0.0174532925199433]],\n ID[\"EPSG\",4267]],\n CONVERSION[\"Transverse Mercator\",\n METHOD[\"Transverse Mercator\",\n ID[\"EPSG\",9807]],\n PARAMETER[\"Latitude of natural origin\",0,\n ANGLEUNIT[\"degree\",0.0174532925199433],\n ID[\"EPSG\",8801]],\n PARAMETER[\"Longitude of natural origin\",-117,\n ANGLEUNIT[\"degree\",0.0174532925199433],\n ID[\"EPSG\",8802]],\n PARAMETER[\"Scale factor at natural origin\",0.9996,\n SCALEUNIT[\"unity\",1],\n ID[\"EPSG\",8805]],\n PARAMETER[\"False easting\",500000,\n LENGTHUNIT[\"metre\",1],\n ID[\"EPSG\",8806]],\n PARAMETER[\"False northing\",0,\n LENGTHUNIT[\"metre\",1],\n ID[\"EPSG\",8807]]],\n CS[Cartesian,2],\n AXIS[\"easting\",east,\n ORDER[1],\n LENGTHUNIT[\"metre\",1]],\n AXIS[\"northing\",north,\n ORDER[2],\n LENGTHUNIT[\"metre\",1]],\n ID[\"EPSG\",26711]]\nData axis to CRS axis mapping: 1,2\nOrigin = (440720.000000000000000,3751320.000000000000000)\nPixel Size = (60.000000000000000,-60.000000000000000)\nMetadata:\n AREA_OR_POINT=Area\nImage Structure Metadata:\n INTERLEAVE=BAND\nCorner Coordinates:\nUpper Left ( 440720.000, 3751320.000) (117d38'28.21\"W, 33d54' 8.47\"N)\nLower Left ( 440720.000, 3745320.000) (117d38'26.75\"W, 33d50'53.66\"N)\nUpper Right ( 446720.000, 3751320.000) (117d34'34.59\"W, 33d54' 9.62\"N)\nLower Right ( 446720.000, 3745320.000) (117d34'33.28\"W, 33d50'54.82\"N)\nCenter ( 443720.000, 3748320.000) (117d36'30.71\"W, 33d52'31.66\"N)\nBand 1 Block=100x81 Type=Byte, ColorInterp=Gray\n Minimum=0.000, Maximum=255.000, Mean=154.621, StdDev=54.251\n Metadata:\n STATISTICS_MAXIMUM=255\n STATISTICS_MEAN=154.6212\n STATISTICS_MINIMUM=0\n STATISTICS_STDDEV=54.250980733624\n STATISTICS_VALID_PERCENT=100\n"

julia> exit()

[Opération terminée]

mathieu17g commented 3 years ago

No more ideas. I clean up and mark the PR as final with the three corrections. I will put the last tests in the issue for later investigations.

mathieu17g commented 2 years ago

You could try in https://github.com/JuliaGeo/GDAL.jl/blob/master/src/error.jl to be stricter, and raise warnings or errors on CE_Warning, now it is set to CE_Failure.

Nothing comes out

Maybe in the DriverManager finalization. I'll have a look...

yeesian commented 2 years ago

Might be worth also updating the PR title to reflect proper finalization of driver management (by deregistering the error handler).

visr commented 2 years ago

I don't fully understand. Now GDAL.jl deregisters the custom error handler on exit, which looks like a good thing.

But why does this PR remove the call to GDAL.gdaldestroydrivermanager()? Isn't that the counterpart to GDAL.gdalallregister() that is called when ArchGDAL loads? Should gdaldestroydrivermanager be called onexit here?

mathieu17g commented 2 years ago

Now we should be good to go :-)

mathieu17g commented 2 years ago

Oops, I rebased to master and got all those new commit reappearing in this pull request. @yeesian I hope I rebased the right way...

yeesian commented 2 years ago

Hmm, might it be easier to bail on this PR and start a new one? Sorry for the churn

mathieu17g commented 2 years ago

Is there a way, on GitHub, to rebase a pull request ? Otherwise how do you delete a PR ? I have not found the button :-)

yeesian commented 2 years ago

Oh we'll be closing the PR rather than deleting it -- but happy to keep the PR open as long as we still find it helpful as a reference

yeesian commented 2 years ago

Is there a way, on GitHub, to rebase a pull request ?

Unfortunately, I don't think it's straightforward if it isn't a "fastforward merge" (from what I've read in https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github#rebasing-and-merging-your-commits)

yeesian commented 2 years ago

Thank you!