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

Polygonize example is broken #187

Open
ConnectedSystems opened this issue Jul 29, 2024 · 5 comments
Open

Polygonize example is broken #187

ConnectedSystems opened this issue Jul 29, 2024 · 5 comments

Comments

@ConnectedSystems
Copy link

Trying out the example here: https://juliageo.org/GeometryOps.jl/stable/source/methods/polygonize#Polygonizing-raster-data

Results in an error

julia> using GeometryOps, GLMakie, GeoMakie

julia> n = 49
49

julia> xs, ys = LinRange(-3, 3, n), LinRange(-3, 3, n);

julia> zs = Makie.peaks(n);

julia> polygons = polygonize(xs, ys, 0.8 .< zs .< 3.2)
ERROR: MethodError: no method matching similar(::Type{Vector{Tuple{Float64, Float64}}}, ::LinRange{Float64, Int64})
@rafaqz
Copy link
Member

rafaqz commented Jul 29, 2024

This is likely getting through tests because we test against OffsetArrays.jl and it pirates similar

JuliaArrays/OffsetArrays.jl#306

I think I've fixed it for poligonize in the testing PR that has taken forever to be merged.

But @jishnub this keeps happening. For the time being we will stop testing against OffsetArrays.jl, it's just not suitable to use in tests.

@jishnub
Copy link

jishnub commented Jul 29, 2024

I'm unsure if I understand the issue here. OffsetArrays defines similar for AbstractUnitRange axes, so a call with LinRange as axes should fail anyway? Does this pass tests with OffsetArrays loaded?

@rafaqz
Copy link
Member

rafaqz commented Jul 29, 2024

It's possible not with this exact range but we've had multiple problems and hacks around it already

@asinghvi17
Copy link
Member

I don't think this is an OffsetArrays issue in this particular case.

@rafaqz
Copy link
Member

rafaqz commented Jul 30, 2024

I think the issue is the code is the way it is to avoid another OffsetArrays.jl issue. There are a bunch of ways for similar to give you an OffsetArray when you don't want it

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 a pull request may close this issue.

4 participants