compas
compas copied to clipboard
Polygon.centroid()
Describe the bug
Polygon.centroid returns a strange value for a polygon with 4 points that is self interesecting (see image),

Using the same points (of the polygon), but cycled differently we get a polygon that is more conventional...

To Reproduce Steps to reproduce the behavior:
- Python script:
from compas.geometry.primitives import Polygon, Point
polygon1 = Polygon([Point(0.000, -5.536, 1977.967), Point(0.000, 144.352, -0.000), Point(0.000, -148.968, -0.000), Point(0.000, 264.624, 1977.967)])
polygon2 = Polygon([Point(0.000, 144.352, -0.000), Point(0.000, 264.624, 1977.967), Point(0.000, -5.536, 1977.967), Point(0.000, -148.968, -0.000)])
print(polygon1.centroid)
print(polygon2.centroid)
>>>>Point(0.000, -471.050, -7031.803)
>>>>Point(0.000, 62.715, 975.434)
Expected behavior I would expect them to have the same centroid.
Screenshots If applicable, add screenshots to help explain your problem.
Desktop (please complete the following information):
- OS: Windows
- Python version 3.8.6
- Python package manager conda
- Compas version 1.0.0
nice investigation :) will have a look...
maybe briefly, just FYI, the recommended way to import compas objects is to always to this from "the second level", which is where the public API is defined. these imports should never change. below that level we sometimes reshuffle things if that is necessary. so in this case the recommended option is
from compas.geometry import Polygon, Point
maybe briefly, just FYI, the recommended way to import compas objects is to always to this from "the second level", which is where the public API is defined. these imports should never change. below that level we sometimes reshuffle things if that is necessary. so in this case the recommended option is
from compas.geometry import Polygon, Point
Thanks for the heads-up. I used to do it "correctly", but the problem for me was that I could never get VSCode to properly auto-complete... I remember discussing this extensively with @yck011522 and @gonzalocasas a while back.
maybe briefly, just FYI, the recommended way to import compas objects is to always to this from "the second level", which is where the public API is defined. these imports should never change. below that level we sometimes reshuffle things if that is necessary. so in this case the recommended option is
from compas.geometry import Polygon, PointThanks for the heads-up. I used to do it "correctly", but the problem for me was that I could never get VSCode to properly auto-complete... I remember discussing this extensively with @yck011522 and @gonzalocasas a while back.
yes, i know, especially with PyLance this is indeed a problem. we are working on that...
about the centroid... the issue is related to how the centroid of the polygon object is calculated.
from compas.geometry import Polygon, Point
from compas.geometry import centroid_points, centroid_polygon
polygon1 = Polygon([Point(0.000, -5.536, 1977.967), Point(0.000, 144.352, -0.000), Point(0.000, -148.968, -0.000), Point(0.000, 264.624, 1977.967)])
polygon2 = Polygon([Point(0.000, 144.352, -0.000), Point(0.000, 264.624, 1977.967), Point(0.000, -5.536, 1977.967), Point(0.000, -148.968, -0.000)])
- using the method of the object
print(polygon1.centroid)
print(polygon2.centroid)
- using
centroid_points
print(centroid_points(polygon1))
print(centroid_points(polygon2))
- using
centroid_polygon
print(centroid_polygon(polygon1))
print(centroid_polygon(polygon2))
the output of 1 and 3 is the same, but i guess you were expecting the outcome of 2 :)
the polygon object uses centroid_polygon in the background to calculate the centroid, which considers the actual polygon surface rather than the corner points to avoid skewed results when there are many points on the boundary that are potentially clustered together and change the location of the centroid. but this method only makes sense if the polygon is not self-intersecting.
unfortunately (apparently), we have not yet established full equivalence between the older functional approach and the newer oo system. this is something that should be resolved in one of the upcoming releases...
Ok thanks for the clarification! I indeed expected the outcome of 2. I also just looked into the api ref. of the centroid_polygon function and there is a big disclaimer for self-intersecting polygons :) so... feel free to close to the issue !

will leave it open as a reminder that the current implementation can lead to unexpected results :)
@gonzalocasas needs to be discussed in the next dev meeting...