search

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

Also dispatch GI.isgeometry on Type. #307

Closed evetion closed 2 years ago

evetion commented 2 years ago

This fixes a small oversight from #290.

rafaqz commented 2 years ago

Was wondering what was going on

rafaqz commented 2 years ago

Well, theres still something odd going on with the tests.

evetion commented 2 years ago

Yep, it's because GDAL v1.4 😢 @visr

rafaqz commented 2 years ago

Pinning 1.3 works for me, that will let my other PRs get merged as well.

visr commented 2 years ago

What was up with GDAL.jl v1.4? If it is just the regular #158 we shouldn't pin it. I tested ArchGDAL locally before making the GDAL release, I believe it was just that.

rafaqz commented 2 years ago

I'm not sure its just that? There are a lot of nothing return values and a method error:

https://github.com/yeesian/ArchGDAL.jl/runs/6903399033?check_suite_focus=true#step:20:296

evetion commented 2 years ago

See https://github.com/yeesian/ArchGDAL.jl/runs/6903398191?check_suite_focus=true

More tests were failing than just a tolerance it seemed, not sure why. And pinning untill we make a release that works with 1.4 seems right?

evetion commented 2 years ago

Failures are due to https://github.com/OSGeo/gdal/commit/1164ba2259457011d6a7be7c7368cf194f529810#diff-360c35bb4393b380a99c8d258b1a8f20dbcc9634e4ccb6e22fe7ad82593892e9L405.

So the manual flag checks implemented in #290 to fix #303 are now not needed anymore.

visr commented 2 years ago

Ha nice find. For all these bool integer returns I guess we need to test > 0. At least GDAL.jl has a new testset now: https://github.com/JuliaGeo/GDAL.jl/commit/4f96fac893ab4a0269322f35fe452186d9309be4