rafaqz / Rasters.jl

Raster manipulation for the Julia language
MIT License
212 stars 36 forks source link

Wrong input for resample gives confusing error ArchGDALBackendException #476

Closed felixcremer closed 1 year ago

felixcremer commented 1 year ago

I am trying to resample an extent to a different CRS so that I don't have to resample all data points and I get this confusing error where it complainst that ArchGDAL should be loaded. ArchGDAL is loaded as visible from [3] and [2]

I would prefer to have a proper MethodError which would give a hint on what other methods are available.

julia> resample(EPSG(4326),lookup(rasfull,:X))
ERROR: `Rasters.jl` requires backends to be loaded externally as of version 0.8. Run `import ArchGDAL` to fix this error.
Stacktrace:
 [1] resample(args::EPSG; kw::Base.Pairs{Symbol, Mapped{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Float64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.Metadata{Rasters.NCDsource, Dict{String, Any}}, ProjString, EPSG, X{Colon}}, Tuple{Symbol}, NamedTuple{(:res,), Tuple{Mapped{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Float64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.Metadata{Rasters.NCDsource, Dict{String, Any}}, ProjString, EPSG, X{Colon}}}}})
   @ Rasters ~/Documents/rqa_cscale/dev/Rasters/src/extensions.jl:4
 [2] resample(x::EPSG, res::Mapped{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Float64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.Metadata{Rasters.NCDsource, Dict{String, Any}}, ProjString, EPSG, X{Colon}}; kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ RastersArchGDALExt ~/Documents/rqa_cscale/dev/Rasters/ext/RastersArchGDALExt/resample.jl:71
 [3] resample(x::EPSG, res::Mapped{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Float64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.Metadata{Rasters.NCDsource, Dict{String, Any}}, ProjString, EPSG, X{Colon}})
   @ RastersArchGDALExt ~/Documents/rqa_cscale/dev/Rasters/ext/RastersArchGDALExt/resample.jl:71
 [4] top-level scope
   @ REPL[66]:1
rafaqz commented 1 year ago

PR? If you can make it so it still gives that warning when ArchGDAL isnt available...

JoshuaBillson commented 1 year ago

I believe this is caused by the following in extensions.jl:

resample(args...; kw...) = throw(BackendException("ArchGDAL"))
warp(args...; kw...) = throw(BackendException("ArchGDAL"))

We're defining these stubs with the goal of raising a BackendException in the event that ArchGDAL isn't loaded, but it seems to also catch the case where the methods are called with incorrect argument types.

I propose that we could fix it like so:

function resample(args...; kw...)
    if @isdefined ArchGDAL
        throw(MethodError(resample, args))
    else
        throw(BackendException("ArchGDAL"))
    end
end

function warp(args...; kw...)
     if @isdefined ArchGDAL
        throw(MethodError(warp, args))
    else
        throw(BackendException("ArchGDAL"))
    end
end
rafaqz commented 1 year ago

@JoshuaBillson that looks like a good solution to me

JoshuaBillson commented 1 year ago

I've opened PR #478 to fix this issue. My proposed solution didn't work as intended, so I ended up making it so that the resample and warp stubs are redefined to throw a MethodError when ArchGDAL is loaded.

Unfortunately, the MethodError displays the signature of the stubs as valid alternatives. I'm not sure there's much we can do to fix this. I tried using Base.delete_method to remove the stubs when ArchGDAL loads, but it doesn't seem to work reliably. We could try defining the stubs with the same type signature as the real methods, but this would be fragile to maintain.

rafaqz commented 1 year ago

Do you not get a duplicate method warning doing that?

JoshuaBillson commented 1 year ago

Yes, but I'm not sure what else we can do. Using @isdefined seems to throw a BackendException even when ArchGDAL is loaded based on the test cases.

rafaqz commented 1 year ago

Yeah but we cant have a warning.

We can check if a method signature defined in the extension exists instead? Im not sure ArchGDAL will be available in that scope anyway