JuliaGeometry / GeometryBasics.jl

Basic Geometry Types
MIT License
165 stars 54 forks source link

`Base.in(::Point, ::Circle)` assumes a circle in L-infinity norm, not L2 norm #108

Closed goretkin closed 3 years ago

goretkin commented 3 years ago

https://github.com/JuliaGeometry/GeometryBasics.jl/blob/2580374cf8c75d9d78f96fe6827cef2e9c1e3184/src/primitives/spheres.jl#L34-L39

is this intentional?

sjkelly commented 3 years ago

This might be an artifact from when we had a HyperSphere. What is the expected behavior?

goretkin commented 3 years ago

The set of points p such that in(p, circle) will look like a square with the current definition. It's not as wrong as it sounds: https://en.wikipedia.org/wiki/Lp_space#/media/File:Vector-p-Norms_qtl1.svg but probably is not the intention.

sjkelly commented 3 years ago

Yeah that is bad.

SimonDanisch commented 3 years ago

Oh, yeah, that's no good!

liuyxpp commented 3 years ago

Moreover, y seems not defined and ox - x leads to a mismatch of dimensions. Does anyone here like to make a PR? If not, I will try to submit one.