JuliaGeometry / GeometryTypes.jl

Geometry types for Julia
Other
67 stars 41 forks source link

Type piracy in minimum, maximum, extrema & isnan #207

Closed yha closed 4 years ago

yha commented 4 years ago

These methods are pirated for StaticArray types:

julia> using StaticArrays
julia> v = SVector.([2,1,3],[1,2,3]);
julia> minimum(v)
2-element SArray{Tuple{2},Int64,1,2} with indices SOneTo(2):
 1
 2
julia> isnan(v[1])
ERROR: MethodError: no method matching isnan(::SArray{Tuple{2},Int64,1,2})
[...]
julia> using GeometryTypes
julia> minimum(v)
ERROR: DimensionMismatch("No precise constructor for SArray{Tuple{2},Int64,1,2} found. Length of input was 1.")
[...]
julia> isnan(v[1])
false

Looking at the code, it looks likes the new minimum is supposed to compute dimension-wise minimum but is failing. I feel these minimum and isnan implementations are a bit questionable for points too. They don't really do what these functions are supposed to be doing. But they definitely shouldn't change the definition for StaticArrays.

SimonDanisch commented 4 years ago

GeometryTypes is deprecated... GeometryBasics doesn't pirate minimum/maximum anymore...But still isnan ... I would mind restricting isnan to only work for GeometryBasic types ;) https://github.com/JuliaGeometry/GeometryBasics.jl/blob/master/src/fixed_arrays.jl#L117

yha commented 4 years ago

Oh, I missed that deprecation. I guess I'll try migrating my code to GeometryBasics, and open a PR there.