MakieOrg / Makie.jl

Interactive data visualizations and plotting in Julia
https://docs.makie.org/stable
MIT License
2.42k stars 313 forks source link

Types not detected correctly from emtpy typed vectors of polygons #4267

Open rafaqz opened 2 months ago

rafaqz commented 2 months ago

This errors because the mesh is using Float32:

polygons = Observable(Polygon{2,Float64}[])
poly(polygons)
push!(polygons[], Polygon([Point(1.0, 2.0), Point(2.0, 3.0), Point(3.0, 2.0)]))
notify(polygons)

Seems there is no way to set the type without passing in objects. So building up the geometries manually is not possible except in Float32.

julia> notify(polygons)
ERROR: MethodError: Cannot `convert` an object of type 
  GeometryBasics.Mesh{2,Float64,GeometryBasics.Ngon{2,Float64,3,Point{2,Float64}},FaceView{GeometryBasics.Ngon{2,Float64,3,Point{2,Float64}},Point{2,Float64},NgonFace{3,OffsetInteger{-1,UInt32}},Array{Point{2
,Float64},1},Array{NgonFace{3,OffsetInteger{-1,UInt32}},1}}} to an object of type 
  GeometryBasics.Mesh{2,Float32,GeometryBasics.Ngon{2,Float32,3,Point{2,Float32}},FaceView{GeometryBasics.Ngon{2,Float32,3,Point{2,Float32}},Point{2,Float32},NgonFace{3,OffsetInteger{-1,UInt32}},Array{Point{2
,Float32},1},Array{NgonFace{3,OffsetInteger{-1,UInt32}},1}}}

Closest candidates are:
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:84
  convert(::Type{T}, ::T) where T<:AbstractArray
   @ Base abstractarray.jl:16
  convert(::Type{T}, ::LinearAlgebra.Factorization) where T<:AbstractArray
   @ LinearAlgebra /opt/julia/share/julia/stdlib/v1.10/LinearAlgebra/src/factorization.jl:108
  ...

Stacktrace:
  [1] setindex!(A::Vector{GeometryBasics.Mesh{…}}, x::GeometryBasics.Mesh{2, Float64, GeometryBasics.Ngon{…}, SimpleFaceView{…}}, i1::Int64)
    @ Base ./array.jl:1021
  [2] _unsafe_copyto!(dest::Vector{GeometryBasics.Mesh{…}}, doffs::Int64, src::Vector{GeometryBasics.Mesh{…}}, soffs::Int64, n::Int64)
    @ Base ./array.jl:299
  [3] unsafe_copyto!
    @ ./array.jl:353 [inlined]
  [4] _copyto_impl!
    @ ./array.jl:376 [inlined]
  [5] copyto!
    @ ./array.jl:363 [inlined]
  [6] copyto!
    @ ./array.jl:385 [inlined]
  [7] copyto_axcheck!
    @ ./abstractarray.jl:1177 [inlined]
  [8] Vector{GeometryBasics.Mesh{2, Float32, GeometryBasics.Ngon{…}, SimpleFaceView{…}}}(x::Vector{GeometryBasics.Mesh{2, Float64, GeometryBasics.Ngon{…}, SimpleFaceView{…}}})
    @ Base ./array.jl:673
  [9] convert(::Type{Vector{GeometryBasics.Mesh{…}}}, a::Vector{GeometryBasics.Mesh{2, Float64, GeometryBasics.Ngon{…}, SimpleFaceView{…}}})
    @ Base ./array.jl:665
 [10] setproperty!(x::Observable{Vector{GeometryBasics.Mesh{…}}}, f::Symbol, v::Vector{GeometryBasics.Mesh{2, Float64, GeometryBasics.Ngon{…}, SimpleFaceView{…}}})
    @ Base ./Base.jl:40
 [11] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/YdEbO/src/Observables.jl:122
 [12] (::Observables.MapCallback)(value::Any)
    @ Observables ~/.julia/packages/Observables/YdEbO/src/Observables.jl:436
 [13] #invokelatest#2
    @ ./essentials.jl:892 [inlined]
 [14] invokelatest
    @ ./essentials.jl:889 [inlined]
 [15] notify
    @ ~/.julia/packages/Observables/YdEbO/src/Observables.jl:206 [inlined]
 [16] #62
    @ ./tuple.jl:627 [inlined]
 [17] BottomRF
    @ ./reduce.jl:86 [inlined]
 [18] afoldl
    @ ./operators.jl:544 [inlined]
 [19] _foldl_impl
    @ ./reduce.jl:68 [inlined]
 [20] foldl_impl
    @ ./reduce.jl:48 [inlined]
 [21] mapfoldl_impl
    @ ./reduce.jl:44 [inlined]
 [22] mapfoldl
    @ ./reduce.jl:175 [inlined]
 [23] foldl
    @ ./reduce.jl:198 [inlined]
 [24] foreach
    @ ./tuple.jl:627 [inlined]
 [25] (::Makie.var"#299#300"{UnionAll, Tuple{Observable{…}}})(kw::@NamedTuple{}, args::Vector{Polygon{2, Float64, P, L, V} where {P<:AbstractPoint{…}, L<:(AbstractVector{…}), V<:AbstractVector{…}}})
    @ Makie ~/.julia/packages/Makie/aG6xp/src/interfaces.jl:186
 [26] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::@Kwargs{})
    @ Base ./essentials.jl:892
 [27] invokelatest(::Any, ::Any, ::Vararg{Any})
    @ Base ./essentials.jl:889
 [28] (::Observables.OnAny)(value::Any)
    @ Observables ~/.julia/packages/Observables/YdEbO/src/Observables.jl:420
 [29] #invokelatest#2
    @ ./essentials.jl:892 [inlined]
 [30] invokelatest
    @ ./essentials.jl:889 [inlined]
 [31] notify(observable::Observables.AbstractObservable)
    @ Observables ~/.julia/packages/Observables/YdEbO/src/Observables.jl:206
 [32] top-level scope
    @ REPL[86]:1
Some type information was truncated. Use `show(err)` to see complete types.

See: https://github.com/MakieOrg/MakieDraw.jl/issues/14

rafaqz commented 2 months ago

This is breaking MakieDraw.jl pretty badly now I've tried to switch things to Float64.

asinghvi17 commented 2 months ago

Looks like a bad type constraint somewhere in convert_arguments to me. Converting number type in a mesh should also not be a problem so I'll ping this to https://github.com/JuliaGeometry/GeometryBasics.jl/pull/173 as a goal there (ideally)...

rafaqz commented 2 months ago

Seems to me there is a if length(polys) > 0 ... check somewhere that switches to Float32, when it should instead look at the type

asinghvi17 commented 2 months ago

haha...

https://github.com/MakieOrg/Makie.jl/blob/017b6968875eb2715b21b6159945d02fbec7f392/src/basic_recipes/poly.jl#L65-L66

rafaqz commented 2 months ago

Lol, yes it is a problem

rafaqz commented 2 months ago

So we just need a method to get N and T from the Polygon type, and use 2 and Float32 if there really is no information available.

Seems like 2 lines of code if I'm not missing something...

kescobo commented 2 months ago

Can you just pull those from eltype or something?

rafaqz commented 2 months ago

Yes, just pass eltype(geometries) to e.g. a _meshfromtype(::Type{<:Polygon{N,T}) method

asinghvi17 commented 2 months ago

I tried the basic fix but it looks like there are still issues elsewhere. Will be a bit longer, I think.