-
Notifications
You must be signed in to change notification settings - Fork 119
Small fixes #1472
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
Small fixes #1472
Changes from all commits
8a783a4
0997040
64b4d60
cac9526
c2bbdc5
67d5410
57ad521
c13e0e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,6 +191,44 @@ | |
| # Methods | ||
| # ============================================================================= | ||
|
|
||
| def isocurve_u(self, u): | ||
| """Compute the isoparametric curve at parameter u. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| u : float | ||
|
|
||
| Returns | ||
| ------- | ||
| :class:`compas.geometry.Circle` | ||
|
|
||
| """ | ||
| origin = self.center | ||
| xaxis = self.point_at(u=u, v=0) - origin | ||
| yaxis = self.point_at(u=u, v=0.25) - origin | ||
| frame = Frame(origin, xaxis, yaxis) | ||
| return Circle(radius=xaxis.length, frame=frame) | ||
|
|
||
| def isocurve_v(self, v): | ||
| """Compute the isoparametric curve at parameter v. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| v : float | ||
|
|
||
| Returns | ||
| ------- | ||
| :class:`compas.geometry.Circle` | ||
|
|
||
| """ | ||
| x = self.point_at(u=0, v=v) | ||
| y = self.point_at(u=0.25, v=v) | ||
| origin = self.center + self.frame.zaxis * (x - self.center).dot(self.frame.zaxis) | ||
| xaxis = x - origin | ||
| yaxis = y - origin | ||
| frame = Frame(origin, xaxis, yaxis) | ||
| return Circle(radius=xaxis.length, frame=frame) | ||
|
Comment on lines
+194
to
+230
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment here about aligning these methods to a unified API that returns
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not possible because the base curve class just defines an api but no functionality |
||
|
|
||
| def point_at(self, u, v, world=True): | ||
| """Construct a point on the sphere. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,9 +112,9 @@ def detect_current_context(): | |
| return "Rhino" | ||
| if compas.is_blender(): | ||
| return "Blender" | ||
| other_contexts = [v for v in ITEM_SCENEOBJECT.keys()] | ||
| if other_contexts: | ||
| return other_contexts[0] | ||
| # other_contexts = [v for v in ITEM_SCENEOBJECT.keys()] | ||
| # if other_contexts: | ||
| # return other_contexts[0] | ||
|
Comment on lines
+115
to
+117
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks dangerous to commit, isn't that going to break stuff, like the viewer?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. viewer and notebook set the context explicitly so they are unaffected. the fact that we return the viewer instead of none is just by coincidence and actually masks bugs and creates unpredictable behavior
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to confirm, I did few tests with viewer too they are not affected
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good, then let's delete instead of comment out |
||
|
|
||
| return None | ||
|
|
||
|
|
||
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.
I understand the logical reason for returning
CircleandLine, but wouldn't make more sense to returnCurveinstances always? That way both_uand_vwould be symmetrical and also this API would be consistent with the one ofSurfaceThere 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.
we don't have a functional general curve
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.
afaik that is also not possible. you need a way to evaluate the curve and this depends on the type of curve it is...