From 71469aa26af2cf6a018c3b717aee3dbf8a21dd73 Mon Sep 17 00:00:00 2001 From: tiemvanderdeure Date: Wed, 4 Sep 2024 19:32:39 +0200 Subject: [PATCH 1/4] call length on AbstractRange --- src/methods/polygonize.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/methods/polygonize.jl b/src/methods/polygonize.jl index da29b1230..c637c97a1 100644 --- a/src/methods/polygonize.jl +++ b/src/methods/polygonize.jl @@ -121,8 +121,8 @@ function _polygonize(f::Base.Callable, xs::AbstractRange, ys::AbstractRange, A:: ybounds = first(ys) - yhalf : step(ys) : last(ys) + yhalf Tx = eltype(xbounds) Ty = eltype(ybounds) - xvec = similar(Vector{Tuple{Tx,Tx}}, xs) - yvec = similar(Vector{Tuple{Ty,Ty}}, ys) + xvec = similar(Vector{Tuple{Tx,Tx}}, length(xs)) + yvec = similar(Vector{Tuple{Ty,Ty}}, length(ys)) for (xind, i) in enumerate(eachindex(xvec)) xvec[i] = xbounds[xind], xbounds[xind+1] end From f3dae6db74bfbc792fea16b14ac6bc91ed46eacf Mon Sep 17 00:00:00 2001 From: tiemvanderdeure Date: Fri, 6 Sep 2024 10:36:48 +0200 Subject: [PATCH 2/4] explicitly set length of xbounds and ybounds --- src/methods/polygonize.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/methods/polygonize.jl b/src/methods/polygonize.jl index c637c97a1..eea66137a 100644 --- a/src/methods/polygonize.jl +++ b/src/methods/polygonize.jl @@ -117,8 +117,8 @@ function _polygonize(f::Base.Callable, xs::AbstractRange, ys::AbstractRange, A:: xhalf = step(xs) / 2 yhalf = step(ys) / 2 # Make bounds ranges first to avoid floating point error making gaps or overlaps - xbounds = first(xs) - xhalf : step(xs) : last(xs) + xhalf - ybounds = first(ys) - yhalf : step(ys) : last(ys) + yhalf + xbounds = range(first(xs) - xhalf, last(xs) + xhalf; length = length(xs) + 1) + ybounds = range(first(ys) - yhalf, last(ys) + yhalf; length = length(ys) + 1) Tx = eltype(xbounds) Ty = eltype(ybounds) xvec = similar(Vector{Tuple{Tx,Tx}}, length(xs)) From 3d5d0719115d91049c7ae6cdc82c2837648f6487 Mon Sep 17 00:00:00 2001 From: tiemvanderdeure Date: Fri, 6 Sep 2024 10:59:13 +0200 Subject: [PATCH 3/4] add some tests --- test/methods/polygonize.jl | 39 +++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/test/methods/polygonize.jl b/test/methods/polygonize.jl index d16a55c0a..abf0af0a6 100644 --- a/test/methods/polygonize.jl +++ b/test/methods/polygonize.jl @@ -1,7 +1,32 @@ using GeometryOps, GeoInterface, Test -import OffsetArrays, DimensionalData, Rasters using ..TestHelpers +import GeometryOps as GO +import GeoInterface as GI + +@testset "Polygonize with xs and ys, without offsetarrays" begin + @test !(@isdefined OffsetArrays) # to make sure this isn't loaded somewhere else + data = rand(1:4, 100, 100) .== 1 + unitrange = 1:100 + steprange = 1:1:100 + steprangelen = range(1, 100; length = 100) + data_mp = polygonize(data) + for range in (unitrange, steprange, steprangelen) + data_mp_range = polygonize(range, range, data) + @test GO.equals(data_mp, data_mp_range) + end + # ideally we'd have a better test to make sure this returns what we think it does + data_mp_range200 = polygonize(2:2:200, 2:2:200, data) + @test length(GI.coordinates(data_mp_range200)) == length(GI.coordinates(data_mp)) + + # this is an example that could throw floating point error + range_floats = -1.333333333333343:0.041666666666666664:0.374999999999986 + data2 = rand(1:4, length(range_floats), length(range_floats)) .== 1 + data_mp_range_floats = polygonize(range_floats, range_floats, data2) +end + +import OffsetArrays, DimensionalData, Rasters + # Missing holes throw a warning, so testing there are # no warnings in a range of randomisation is one way to test # that things are working, without testing explicit return values @@ -68,3 +93,15 @@ end @test GI.crs(evil_mp) == GI.crs(evil) end end + +@testset "Polygonize with xs and ys, with offsetarrays" begin + data = rand(1:4, 100, 100) .== 1 + unitrange = 1:100 + steprange = 1:1:100 + steprangelen = range(1, 100; length = 100) + data_mp = polygonize(data) + for range in (unitrange, steprange, steprangelen) + data_mp_range = polygonize(range, range, data) + @test GO.equals(data_mp, data_mp_range) + end +end From 395193c28011b118d687bb85b01806da901c8ab3 Mon Sep 17 00:00:00 2001 From: tiemvanderdeure Date: Fri, 6 Sep 2024 11:58:24 +0200 Subject: [PATCH 4/4] use range with explicit step for bounds --- src/methods/polygonize.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/methods/polygonize.jl b/src/methods/polygonize.jl index eea66137a..49e7c5f6c 100644 --- a/src/methods/polygonize.jl +++ b/src/methods/polygonize.jl @@ -117,8 +117,8 @@ function _polygonize(f::Base.Callable, xs::AbstractRange, ys::AbstractRange, A:: xhalf = step(xs) / 2 yhalf = step(ys) / 2 # Make bounds ranges first to avoid floating point error making gaps or overlaps - xbounds = range(first(xs) - xhalf, last(xs) + xhalf; length = length(xs) + 1) - ybounds = range(first(ys) - yhalf, last(ys) + yhalf; length = length(ys) + 1) + xbounds = range(first(xs) - xhalf; step = step(xs), length = length(xs) + 1) + ybounds = range(first(ys) - yhalf; step = step(ys), length = length(ys) + 1) Tx = eltype(xbounds) Ty = eltype(ybounds) xvec = similar(Vector{Tuple{Tx,Tx}}, length(xs))