Skip to content

Bounding box API #88

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

Closed
juliohm opened this issue Sep 22, 2020 · 4 comments
Closed

Bounding box API #88

juliohm opened this issue Sep 22, 2020 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@juliohm
Copy link
Member

juliohm commented Sep 22, 2020

First let me say that the new documentation is really helpful. I am adding GeometryBasics.jl as a dependency in GeoStats.jl soon given that things are getting in place nicely here.

I've noticed that the boundingboxes.jl file contains axis-aligned bounding box algorithms for the different primitives and geometries. May I submit a PR replacing the Rect constructors by an actual boundbox function to be exported by the package? This way we separate the algorithm of bounding box from the actual constructor of the rectangle primitive. In the future, we could have alternative algorithms to the boundbox like for example convex and concave hulls convhull and conchull.

@juliohm juliohm added the enhancement New feature or request label Sep 22, 2020
@juliohm juliohm self-assigned this Sep 22, 2020
@SimonDanisch
Copy link
Member

That'd be kind of cool!
Although, I do like, that one can so easily specify the element type by using Rect...
Also, we'd need to think about the name, to not start lots of conflicts!

@juliohm
Copy link
Member Author

juliohm commented Sep 23, 2020

Wouldn't it make sense to inherit the element type of the object inside the bounding box? What is the use case you have in mind where the bounding box will have a different element type? By element type you mean coordinate type, correct? Also, I think it would still be easy to use the Rect constructors to convert from one Rect to another with a different coordinate type after the bounding box is computed. I think it is beneficial overall to separate these things. I will prepare a PR accordingly and we can review from there :)

@SimonDanisch
Copy link
Member

Yeah, it's not a big thing ;) Inheriting element type seems fine

juliohm added a commit that referenced this issue Sep 24, 2020
juliohm added a commit that referenced this issue Sep 25, 2020
@juliohm
Copy link
Member Author

juliohm commented Sep 25, 2020

Fixed in branch cleanup.

@juliohm juliohm closed this as completed Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants