-
Notifications
You must be signed in to change notification settings - Fork 7
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
Added function create_offset_polygons_with_holes_2
#35
Conversation
@@ -155,6 +155,63 @@ def create_offset_polygons_2(points, offset) -> list[Polygon]: | |||
return [Polygon(points.tolist()) for points in offset_polygons] | |||
|
|||
|
|||
def create_offset_polygons_with_holes_2(points, holes, offset) -> list[Tuple[Polygon, list[Polygon]]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should have asked this before, but i guess _2
means "2D"? perhaps this should be mentioned in the docstrings...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, i've added it to the docstrings. I've use "_2" as the cgal functions are named like that as well
Raises | ||
------ | ||
ValueError | ||
If the normal of the polygon is not [0, 0, 1]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be a unit normal, or vertical up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be vertical up, i've adjusted the docstrings
------ | ||
ValueError | ||
If the normal of the polygon is not [0, 0, 1]. | ||
If the normal of a hole is not [0, 0, -1]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be a unit normal, or vertical down?
|
||
H = [] | ||
for i, hole in enumerate(holes): | ||
points = list(hole) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the conversion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, not needed
hole = np.asarray(points, dtype=np.float64) | ||
H.append(hole) | ||
|
||
offset = float(offset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the conversion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in any case, would do this at the beginning of the algorithm. process all input params (if necessary) at the start...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, it is not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Added function
create_offset_polygons_with_holes_2
along with some code refactorings onstraight_skeleton_2.cpp
.