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

Intersection methods that accepts tuples. #1323

Open
petrasvestartas opened this issue Apr 15, 2024 · 3 comments
Open

Intersection methods that accepts tuples. #1323

petrasvestartas opened this issue Apr 15, 2024 · 3 comments
Assignees
Labels

Comments

@petrasvestartas
Copy link
Contributor

petrasvestartas commented Apr 15, 2024

Describe the bug
I doubt it is a bug, but still would like to ask.
In several intersection methods of compas, input parameters are either a tuple or a primitive geometry object, but it works only with a tuple. For example:

p0, p1 = intersection_sphere_line((sphere.base, sphere.radius), line)

But this does not work, since the intersection method tries to unpack the sphere:
p0, p1 = intersection_sphere_line(sphere, line)

To Reproduce
Above.

Expected behavior
Preferred way would be to work with a primitive rather than a tuple.

Desktop (please complete the following information):

  • OS: Ubuntu
  • Python version 3.9.10
  • Python pip installed compas version from git clone.
@tomvanmele
Copy link
Member

@gonzalocasas this problem still exists in many places. perhaps we should remove the explicit reference to equivalent geometry objects from the docs. the lower level function API then explicitly uses the minimal mathematical representation of these objects, and in the higher level object API, we do this conversion behind the scenes automatically...

also, i think we should rename the function to always list the lower dimensional entity first.

@tomvanmele tomvanmele self-assigned this Apr 17, 2024
@tomvanmele tomvanmele added the bug label Apr 17, 2024
@chenkasirer
Copy link
Member

chenkasirer commented Oct 29, 2024

@tomvanmele
This came up again.
Just to make sure I understand

perhaps we should remove the explicit reference to equivalent geometry objects from the docs.

You mean rename the args e.g. sphere to point_radius?

in the higher level object API, we do this conversion behind the scenes automatically...

Where is the higher level API? you mean something like a new Sphere.intersection_line(line)?

also, i think we should rename the function to always list the lower dimensional entity first.

intersection_line_sphere ?

@tomvanmele
Copy link
Member

some lower level functions used to refer explicitly to objects like circle and sphere as valid input. if that is still the case, we should remove those references, and instead make it clear that when the input is, for example, a circle, the user should provide it as (point, normal, radius).

i indeed think that all of the lower level functions should be accessible through the higher level API. to avoid repetition, i would suggest to always make such functionality available on the lower dimensional object. for example

  • line.intersection_plane(plane)
  • line.intersection_box(box)
  • line.intersection_surface(surface)

even better would be to make an intersection class that groups all of these...

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

No branches or pull requests

3 participants