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

item as kwarg #1353

Merged
merged 18 commits into from
Jun 11, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Changed `compas.tolerance.Tolerance` to a singleton, to ensure having only library-wide tolerance values.
* Updated `compas_rhino.conversions.point_to_compas` to allow for `Rhino.Geometry.Point` as input.
* Changed `compas.datastructures.Tree.print_hierarchy` to `compas.datastructures.Tree.__str__`.
* Changed `compas.scene.SceneObject.__init__` to accept `item` as kwarg.
* Fixed `compas.geometry.bbox_numpy.minimum_volume_box` to avoid `numpy.linalg.LinAlgError`.

### Removed
Expand Down
29 changes: 18 additions & 11 deletions src/compas/scene/geometryobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,24 @@ class GeometryObject(SceneObject):
linecolor = ColorAttribute()
surfacecolor = ColorAttribute()

def __init__(self, geometry, **kwargs):
super(GeometryObject, self).__init__(item=geometry, **kwargs)
self.geometry = geometry
self.pointcolor = kwargs.get("pointcolor", self.color)
self.linecolor = kwargs.get("linecolor", self.color)
self.surfacecolor = kwargs.get("surfacecolor", self.color)
self.pointsize = kwargs.get("pointsize", 1.0)
self.linewidth = kwargs.get("linewidth", 1.0)
self.show_points = kwargs.get("show_points", False)
self.show_lines = kwargs.get("show_lines", True)
self.show_surfaces = kwargs.get("show_surfaces", True)
def __init__(self, pointcolor=None, linecolor=None, surfacecolor=None, pointsize=1.0, linewidth=1.0, show_points=False, show_lines=True, show_surfaces=True, **kwargs):
super(GeometryObject, self).__init__(**kwargs)
self.pointcolor = pointcolor or self.color
self.linecolor = linecolor or self.color
self.surfacecolor = surfacecolor or self.color
self.pointsize = pointsize
self.linewidth = linewidth
self.show_points = show_points
self.show_lines = show_lines
self.show_surfaces = show_surfaces
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So after adjustment, any __init__ of subclasses should only care about additional parameters that its parent classes do not have, the data item will be passed along automatically within **kwargs and eventualy received by the root __init__ of SceneObject


@property
def geometry(self):
return self.item
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we need alias we create a property that points to self.item instead of creating another attribute.


@geometry.setter
def geometry(self, geometry):
self.item = geometry

def draw(self):
"""Draw the geometry. Implemented by child classes."""
Expand Down
14 changes: 3 additions & 11 deletions src/compas/scene/graphobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,9 @@ class GraphObject(SceneObject):
nodecolor = ColorDictAttribute()
edgecolor = ColorDictAttribute()

def __init__(self, graph, **kwargs):
super(GraphObject, self).__init__(item=graph, **kwargs)
self._graph = None
def __init__(self, **kwargs):
Licini marked this conversation as resolved.
Show resolved Hide resolved
super(GraphObject, self).__init__(**kwargs)
self._node_xyz = None
self.graph = graph
self.nodecolor = kwargs.get("nodecolor", self.color)
self.edgecolor = kwargs.get("edgecolor", self.color)
self.nodesize = kwargs.get("nodesize", 1.0)
Expand All @@ -60,13 +58,7 @@ def __init__(self, graph, **kwargs):

@property
def graph(self):
return self._graph

@graph.setter
def graph(self, graph):
Copy link
Contributor Author

@Licini Licini May 22, 2024

Choose a reason for hiding this comment

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

These setters I removed for now, because item in SceneObject is currently read-only

self._graph = graph
self._transformation = None
self._node_xyz = None
return self.item

@property
def transformation(self):
Expand Down
8 changes: 3 additions & 5 deletions src/compas/scene/meshobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,9 @@ class MeshObject(SceneObject):
edgecolor = ColorDictAttribute()
facecolor = ColorDictAttribute()

def __init__(self, mesh, **kwargs):
super(MeshObject, self).__init__(item=mesh, **kwargs)
self._mesh = None
def __init__(self, **kwargs):
Licini marked this conversation as resolved.
Show resolved Hide resolved
super(MeshObject, self).__init__(**kwargs)
self._vertex_xyz = None
self.mesh = mesh
self.vertexcolor = kwargs.get("vertexcolor", self.contrastcolor)
self.edgecolor = kwargs.get("edgecolor", self.contrastcolor)
self.facecolor = kwargs.get("facecolor", self.color)
Expand All @@ -72,7 +70,7 @@ def __init__(self, mesh, **kwargs):

@property
def mesh(self):
return self._mesh
return self.item

@mesh.setter
def mesh(self, mesh):
Expand Down
2 changes: 1 addition & 1 deletion src/compas/scene/scene.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def add(self, item, parent=None, **kwargs):
if kwargs["context"] != self.context:
raise Exception("Object context should be the same as scene context: {} != {}".format(kwargs["context"], self.context))
del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error
sceneobject = SceneObject(item, context=self.context, **kwargs) # type: ignore
sceneobject = SceneObject(item=item, context=self.context, **kwargs) # type: ignore
super(Scene, self).add(sceneobject, parent=parent)
return sceneobject

Expand Down
11 changes: 6 additions & 5 deletions src/compas/scene/sceneobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import compas.datastructures # noqa: F401
import compas.geometry # noqa: F401
from compas.colors import Color
from compas.data import Data
from compas.datastructures import TreeNode
from compas.geometry import Transformation

Expand Down Expand Up @@ -82,13 +83,13 @@ class SceneObject(TreeNode):

color = ColorAttribute()

def __new__(cls, item, **kwargs):
def __new__(cls, item=None, **kwargs):
sceneobject_cls = get_sceneobject_cls(item, **kwargs)
return super(SceneObject, cls).__new__(sceneobject_cls)

def __init__(self, item, name=None, color=None, opacity=1.0, show=True, frame=None, transformation=None, context=None, **kwargs): # fmt: skip
# type: (compas.geometry.Geometry | compas.datastructures.Datastructure, str | None, compas.colors.Color | None, float, bool, compas.geometry.Frame | None, compas.geometry.Transformation | None, str | None, dict) -> None
name = name or item.name
def __init__(self, item=None, name=None, color=None, opacity=1.0, show=True, frame=None, transformation=None, context=None, **kwargs): # fmt: skip
Licini marked this conversation as resolved.
Show resolved Hide resolved
# type: (compas.data.Data | None, str | None, compas.colors.Color | None, float, bool, compas.geometry.Frame | None, compas.geometry.Transformation | None, str | None, dict) -> None
name = item.name if isinstance(item, Data) and name is None else name
super(SceneObject, self).__init__(name=name, **kwargs)
# the scene object needs to store the context
# because it has no access to the tree and/or the scene before it is added
Expand Down Expand Up @@ -237,7 +238,7 @@ def add(self, item, **kwargs):
if kwargs["context"] != self.context:
raise Exception("Child context should be the same as parent context: {} != {}".format(kwargs["context"], self.context))
del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error
sceneobject = SceneObject(item, context=self.context, **kwargs) # type: ignore
sceneobject = SceneObject(item=item, context=self.context, **kwargs) # type: ignore

super(SceneObject, self).add(sceneobject)
return sceneobject
Expand Down
14 changes: 3 additions & 11 deletions src/compas/scene/volmeshobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,9 @@ class VolMeshObject(SceneObject):
facecolor = ColorDictAttribute()
cellcolor = ColorDictAttribute()

def __init__(self, volmesh, **kwargs):
super(VolMeshObject, self).__init__(item=volmesh, **kwargs)
self._volmesh = None
def __init__(self, **kwargs):
Licini marked this conversation as resolved.
Show resolved Hide resolved
super(VolMeshObject, self).__init__(**kwargs)
self._vertex_xyz = None
self.volmesh = volmesh
self.vertexcolor = kwargs.get("vertexcolor", self.color)
self.edgecolor = kwargs.get("edgecolor", self.color)
self.facecolor = kwargs.get("facecolor", self.color)
Expand All @@ -82,13 +80,7 @@ def __init__(self, volmesh, **kwargs):

@property
def volmesh(self):
return self._volmesh

@volmesh.setter
def volmesh(self, volmesh):
self._volmesh = volmesh
self._transformation = None
self._vertex_xyz = None
return self.item

@property
def transformation(self):
Expand Down
15 changes: 1 addition & 14 deletions src/compas_rhino/scene/boxobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,7 @@


class RhinoBoxObject(RhinoSceneObject, GeometryObject):
"""Scene object for drawing box shapes.

Parameters
----------
box : :class:`compas.geometry.Box`
A COMPAS box.
**kwargs : dict, optional
Additional keyword arguments.

"""

def __init__(self, box, **kwargs):
super(RhinoBoxObject, self).__init__(geometry=box, **kwargs)
self.box = box
"""Scene object for drawing box shapes."""

def draw(self):
"""Draw the box associated with the scene object.
Expand Down
12 changes: 1 addition & 11 deletions src/compas_rhino/scene/brepobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,7 @@


class RhinoBrepObject(RhinoSceneObject, GeometryObject):
"""A scene object for drawing a RhinoBrep.

Parameters
----------
brep : :class:`compas_rhino.geometry.RhinoBrep`
The Brep to draw.

"""

def __init__(self, brep, **kwargs):
super(RhinoBrepObject, self).__init__(geometry=brep, **kwargs)
"""A scene object for drawing a RhinoBrep."""
Copy link
Contributor Author

@Licini Licini May 22, 2024

Choose a reason for hiding this comment

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

In many cases the __init__ function of subclass can be entirely omitted, because the don't do anything else than passing in the positional argument using a different name.


def draw(self):
"""Bakes the Brep into the current document
Expand Down
14 changes: 1 addition & 13 deletions src/compas_rhino/scene/capsuleobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,7 @@


class RhinoCapsuleObject(RhinoSceneObject, GeometryObject):
"""Scene object for drawing capsule shapes.

Parameters
----------
capsule : :class:`compas.geometry.Capsule`
A COMPAS capsule.
**kwargs : dict, optional
Additional keyword arguments.

"""

def __init__(self, capsule, **kwargs):
super(RhinoCapsuleObject, self).__init__(geometry=capsule, **kwargs)
"""Scene object for drawing capsule shapes."""

def draw(self):
"""Draw the capsule associated with the scene object.
Expand Down
8 changes: 4 additions & 4 deletions src/compas_rhino/scene/meshobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class RhinoMeshObject(RhinoSceneObject, MeshObject):

Parameters
----------
mesh : :class:`compas.datastructures.Mesh`
A COMPAS mesh.
disjoint : bool, optional
Draw the faces of the mesh disjointed.
**kwargs : dict, optional
Additional keyword arguments.

Expand All @@ -43,8 +43,8 @@ class RhinoMeshObject(RhinoSceneObject, MeshObject):

"""

def __init__(self, mesh, disjoint=False, **kwargs):
super(RhinoMeshObject, self).__init__(mesh=mesh, **kwargs)
def __init__(self, disjoint=False, **kwargs):
super(RhinoMeshObject, self).__init__(**kwargs)
self.disjoint = disjoint
self._guid_mesh = None
self._guids_faces = None
Expand Down
Loading