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

Reconsider Point behavior #22

Closed
juliohm opened this issue Mar 10, 2017 · 6 comments · Fixed by #34
Closed

Reconsider Point behavior #22

juliohm opened this issue Mar 10, 2017 · 6 comments · Fixed by #34
Labels

Comments

@juliohm
Copy link
Member

juliohm commented Mar 10, 2017

The following behavior seems super odd:

using GeometricalPredicates

p = Point(1,1)
gety(p)
# 1.5

I always disliked this min_coord=1, max_coord=2 constraint, it is very unpleasant both mathematically speaking and programming-wise, but I believe there is a very good numerical reason for it.

This issue, however, is about the odd behavior of the point type regardless of numerics, can you please reconsider the design?

@skariel
Copy link
Contributor

skariel commented Mar 11, 2017

this is a bug, I was not aware of it. I'll fix it

@skariel skariel added the bug label Mar 11, 2017
@skariel
Copy link
Contributor

skariel commented Sep 22, 2018

actually this is not a bug-- as thee docs say the coordinates have to be min_coord <= x <= max_coord and min_coord = 1.0 + eps o obviously using x=1 is outside of this spec...

@skariel skariel closed this as completed Sep 22, 2018
@skariel
Copy link
Contributor

skariel commented Sep 22, 2018

oops, I was referring to the voronoidelaunay docs... this is really a bug, reopening now

@skariel skariel reopened this Sep 22, 2018
@skariel
Copy link
Contributor

skariel commented Sep 22, 2018

The code seems good, it is like a bug in Julia or something, I'll investigate...

@skariel
Copy link
Contributor

skariel commented Sep 22, 2018

no bug with Julia of course :) the problem is that you are calling Point with integers and the the following method gets called down the line: Point2D(peanokey::Int64, bits::Int64=peano_2D_bits)

I'm not sure this doesn't make sense, it is certainly not a bug, just confusing

@skariel skariel added api and removed bug labels Sep 22, 2018
@dkarrasch
Copy link
Collaborator

Could it be that we should have

Point(x::Real, y::Real) = Point2D(float(x), float(y))
Point(x::Real, y::Real, z::Real) = Point3D(float(x), float(y), float(z))

? Point has only these two methods, and there is no confusion with the peano-key functions Point2D and Point3D.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants