Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* Added `compas.scene.Scene.add_group()` for adding group.
* Added `compas.scene.Group.add_from_list()` for adding a list of items to a group.
* Added implementation for `compas.geometry.SphericalSurface.isocurve_u`.
* Added implementation for `compas.geometry.SphericalSurface.isocurve_v`.
* Added implementation for `compas.geometry.CylindricalSurface.isocurve_u`.
* Added implementation for `compas.geometry.CylindricalSurface.isocurve_v`.

### Changed

* Fixed error in `circle_to_compas` from Rhino.
* Fixed Rhino to Rhino brep serialization.
* Upated `compas.scene.Group.add()` to pass on group kwargs as default for child items.
* Fixed bug in context detection, which wrongly defaults to `Viewer` instead of `None`.
* Fixed bug in calculation of `compas.geometry.Polyhedron.edges` if geometry is computed using numpy.
* Fixed bug in `Grpah.from_pointcloud` which uses degree parameter wrongly.

### Removed

Expand Down
20 changes: 18 additions & 2 deletions src/compas/datastructures/graph/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
from ast import literal_eval
from itertools import combinations
from random import sample
from random import choice
from random import shuffle

import compas

Expand Down Expand Up @@ -159,7 +161,13 @@ def __from_data__(cls, data):
graph._max_node = data.get("max_node", graph._max_node)
return graph

def __init__(self, default_node_attributes=None, default_edge_attributes=None, name=None, **kwargs):
def __init__(
self,
default_node_attributes=None,
default_edge_attributes=None,
name=None,
**kwargs
): # fmt: skip
super(Graph, self).__init__(kwargs, name=name)
self._max_node = -1
self.node = {}
Expand Down Expand Up @@ -375,8 +383,16 @@ def from_pointcloud(cls, cloud, degree=3):
graph = cls()
for x, y, z in cloud:
graph.add_node(x=x, y=y, z=z)
nodes = list(graph.nodes())
for u in graph.nodes():
for v in graph.node_sample(size=degree):
shuffle(nodes)
for v in nodes:
if v == u:
continue
if graph.degree(v) == degree:
continue
if graph.degree(u) == degree:
break
graph.add_edge(u, v)
return graph

Expand Down
9 changes: 8 additions & 1 deletion src/compas/datastructures/mesh/mesh.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,14 @@ def __from_data__(cls, data):

return mesh

def __init__(self, default_vertex_attributes=None, default_edge_attributes=None, default_face_attributes=None, name=None, **kwargs): # fmt: skip
def __init__(
self,
default_vertex_attributes=None,
default_edge_attributes=None,
default_face_attributes=None,
name=None,
**kwargs
): # fmt: skip
super(Mesh, self).__init__(kwargs, name=name)
self._max_vertex = -1
self._max_face = -1
Expand Down
4 changes: 3 additions & 1 deletion src/compas/geometry/polyhedron.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,9 @@ def faces(self, faces):
def edges(self):
seen = set()
for face in self.faces:
for u, v in pairwise(face + face[:1]):
for i in range(-1, len(face) - 1):
u = face[i]
v = face[i + 1]
if (u, v) not in seen:
seen.add((u, v))
seen.add((v, u))
Expand Down
34 changes: 34 additions & 0 deletions src/compas/geometry/surfaces/cylindrical.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from compas.geometry import Circle
from compas.geometry import Frame
from compas.geometry import Line
from compas.geometry import Point
from compas.geometry import Vector

Expand Down Expand Up @@ -162,6 +163,39 @@
# Methods
# =============================================================================

def isocurve_u(self, u):
"""Compute the isoparametric curve at parameter u.

Parameters
----------
u : float

Returns
-------
:class:`compas.geometry.Line`

"""
base = self.point_at(u=u, v=0.5)
return Line.from_point_direction_length(base, self.frame.zaxis, 1.0)

Check warning on line 179 in src/compas/geometry/surfaces/cylindrical.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/cylindrical.py#L178-L179

Added lines #L178 - L179 were not covered by tests

def isocurve_v(self, v):
"""Compute the isoparametric curve at parameter v.

Parameters
----------
v : float

Returns
-------
:class:`compas.geometry.Circle`

"""
point = self.center + self.frame.zaxis * v
xaxis = self.frame.xaxis
yaxis = self.frame.yaxis
frame = Frame(point, xaxis, yaxis)
return Circle(radius=self.radius, frame=frame)

Check warning on line 197 in src/compas/geometry/surfaces/cylindrical.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/cylindrical.py#L193-L197

Added lines #L193 - L197 were not covered by tests
Comment on lines +166 to +197
Copy link
Copy Markdown
Member

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 Circle and Line, but wouldn't make more sense to return Curve instances always? That way both _u and _v would be symmetrical and also this API would be consistent with the one of Surface

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member Author

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...


def point_at(self, u, v, world=True):
"""Compute a point on the surface at the given parameters.

Expand Down
38 changes: 38 additions & 0 deletions src/compas/geometry/surfaces/spherical.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check warning on line 210 in src/compas/geometry/surfaces/spherical.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/spherical.py#L206-L210

Added lines #L206 - L210 were not covered by tests

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)

Check warning on line 230 in src/compas/geometry/surfaces/spherical.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/spherical.py#L224-L230

Added lines #L224 - L230 were not covered by tests
Comment on lines +194 to +230
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 Curve always

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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.

Expand Down
2 changes: 1 addition & 1 deletion src/compas/geometry/surfaces/surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ def isocurve_v(self, v):

Returns
-------
:class:`compas_occ.geometry.Curve`
:class:`compas.geometry.Curve`

"""
raise NotImplementedError
Expand Down
2 changes: 1 addition & 1 deletion src/compas/rpc/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
try:
idict = json.loads(args[0], cls=DataDecoder)
except (IndexError, TypeError):
odict["error"] = "API methods require a single JSON encoded dictionary as input.\n" "For example: input = json.dumps({'param_1': 1, 'param_2': [2, 3]})"
odict["error"] = "API methods require a single JSON encoded dictionary as input.\nFor example: input = json.dumps({'param_1': 1, 'param_2': [2, 3]})"

Check warning on line 125 in src/compas/rpc/dispatcher.py

View check run for this annotation

Codecov / codecov/patch

src/compas/rpc/dispatcher.py#L125

Added line #L125 was not covered by tests

else:
self._call(function, idict, odict)
Expand Down
6 changes: 3 additions & 3 deletions src/compas/scene/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, then let's delete instead of comment out


return None

Expand Down
2 changes: 1 addition & 1 deletion tests/compas/datastructures/test_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_graph_from_pointcloud():
graph = Graph.from_pointcloud(cloud=cloud, degree=3)
assert graph.number_of_nodes() == len(cloud)
for node in graph.nodes():
assert graph.degree(node) >= 3
assert graph.degree(node) <= 3


# ==============================================================================
Expand Down
4 changes: 3 additions & 1 deletion tests/compas/geometry/test_polyhedron.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ def test_polyhedron():
assert polyhedron.vertices == vertices
assert polyhedron.faces == faces
assert polyhedron.name == name
assert all(u < len(polyhedron.vertices) and v < len(polyhedron.vertices) for u, v in polyhedron.edges)
assert all(v < len(polyhedron.vertices) for face in polyhedron.faces for v in face)
assert polyhedron.points == vertices
assert polyhedron.lines == [(a, b) for a, b in pairwise(vertices + vertices[:1])]
assert polyhedron.lines == [(a, b) for a, b in pairwise(vertices[-1:] + vertices)]
assert polyhedron.points[0] == vertices[0]
assert polyhedron.points[-1] != polyhedron.points[0]
Comment thread
tomvanmele marked this conversation as resolved.
Loading