JuliaGeometry / GeometryBasics.jl

Basic Geometry Types
MIT License
168 stars 54 forks source link

`AbstractMesh` is just `AbstractVector` #54

Closed rdeits closed 4 years ago

rdeits commented 4 years ago

I'm working on porting MeshCat.jl from GeometryTypes to GeometryBasics, and I just ran into an interesting method ambiguity that turned out to be due to the fact that AbstractMesh is exactly AbstractVector:

julia> AbstractMesh
AbstractArray{Element,1} where Element

julia> AbstractMesh == AbstractVector
true

This makes it impossible to write a function that dispatches on a general AbstractVector vs. an AbstractMesh. It also means that any function written for an AbstractMesh is actually claiming to support any kind of vector of any element, which seems wrong.

It's not a blocker for my work, but I think it's likely to be a minor source of annoyance going forward. Would it be possible to put some kind of type restriction on the element of AbstractMesh? Anything more restrictive than Any would fix the issue and make AbstractMesh a usable type.

rdeits commented 4 years ago

Or, if determining a meaningful element type restriction is hard, what about doing:

abstract type AbstractMesh{Element} <: AbstractVector{Element} end

which would fix the root issue without limiting the possible element types.

Sov-trotter commented 4 years ago

I am working on a similar thing for Shapefile.jl JuliaGeo/Shapefile.jl#39 And while creating a type for MultiPatch type, that is actually a Mesh, realized that GB doesn't currently have a metatype for Mesh types. So I tried to write one similar to the one here JuliaGeometry/GeometryBasics.jl#52 Something like this, @meta_type(Mesh, mesh, AbstractVector, P) and it works. So doesn't that automatically suggest, AbstractVector supertypes Mesh type?

SimonDanisch commented 4 years ago

Oh, I thought I'm restricting the Element type... That's definitely bad :D Also, 99% of the methods should like use Mesh for typing the arguments