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

Laybug Face3D .area error after duplicate() > to_dict() > from_dict() #404

Open
ed-p-may opened this issue May 20, 2024 · 0 comments
Open

Comments

@ed-p-may
Copy link
Contributor

Hi - I'm reposing this issue from the ladybug-tools forum here as well for reference. I recently had to revisit this issue for another project, and I'm pretty sure this is what is causing the issue:

> Original Post

When Face3D.__copy__() is called, it passes self.vertices in as the boundary argument for the new Face3D.__init__()

def __copy__(self):
     _new_face = Face3D(**self.vertices**, self.plane) #<----- HERE
     self._transfer_properties(_new_face)
     _new_face._holes = self._holes
     _new_face._polygon2d = self._polygon2d
     _new_face._mesh2d = self._mesh2d
     _new_face._mesh3d = self._mesh3d
     return _new_face

I believe this is incorrect and source of the issue. As stated in the docstring for the vertices property:

@property
def vertices(self):
    """Tuple of all vertices in this face.
    Note that, in the case of a face with holes, some vertices will be repeated
    since this property effectively traces out a single boundary around the
    whole shape, winding inward to cut out the holes.
    """
    ...

whereas the boundary argument is actually supposed to be only the outer boundary (without holes), according to its docstring:

@property
def boundary(self):
    """Tuple of vertices on the boundary of this face.
    For most Face3D objects, this will be identical to the vertices property.
    However, when the Face3D has holes within it, this property stores
    The outer boundary of the shape.
    """
    ...

So it appears that when duplicating a Face3D with holes in it, the holes area incorrectly included as part of the boundary which is passed in. Then, when the holes are set explicitly by the line _new_face._holes = self._holes they end up being doubled up.


For my test case, I have found that by just adding the line _new_face._boundary = self._boundary to __copy__, the error outlined above in the .area property appears to go away and I get the result I was expecting. But I’m not sure if that is a good enough fix to resolve all the cases where this issue might occur? There are a lot of various polygon methods on Face3D that I have not looked at very closely.

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

No branches or pull requests

1 participant