New EntityList functions to construct and determine the properties of EntityLists #528
Conversation
…ns, as well as a Region constructor to generate a Region when given points
aidanc-ansys
requested review from
jgsdavies,
ravi-ansys,
JackB-Ansys,
matthewAnsys and
james-packer
as code owners
aidanc-ansys
added
bug
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #528 +/- ##
==========================================
+ Coverage 88.62% 89.00% +0.37%
==========================================
Files 21 21
Lines 2479 2546 +67
==========================================
+ Hits 2197 2266 +69
+ Misses 282 280 -2 🚀 New features to boost your workflow:
|
|
@aidanc-ansys, not directly related to this PR, but it's worth being aware of the (slightly confusingly named) return_entity_list method of geometry_fitting (https://motorcad.docs.pyansys.com/version/dev/methods/_autosummary_geometry_fitting/ansys.motorcad.core.geometry_fitting.return_entity_list.html), which performs a related function. The geometry_fitting one takes a list of points, and returns a combination of arcs and lines that fits these points within a certain tolerance. |
src/ansys/motorcad/core/geometry.py
Outdated
| @@ -164,6 +164,65 @@ def from_coordinate_list(cls): | |||
| """ | |||
| pass | |||
|
|
|||
| @classmethod | |||
| def from_points_linear( | |||
There was a problem hiding this comment.
Maybe rename to include polygon? I Quite like Region.polygon()
What was your thinking for this name?
There was a problem hiding this comment.
I also feel most of this should belong to the EntityList class, This is where the function name Polygon would work nicely
src/ansys/motorcad/core/geometry.py
Outdated
| xcenter = sum(list(point.x for point in points)) / len(points) | ||
| ycenter = sum(list(point.y for point in points)) / len(points) | ||
|
|
||
| def to_relative_coords(point): |
There was a problem hiding this comment.
I would make to_relative_coords and to_real_coords part of the Coordinate class, I expect these functions will be useful outside of here.
src/ansys/motorcad/core/geometry.py
Outdated
| for i in range(0, len(self._entities)) | ||
| ) | ||
| else: | ||
| return False |
There was a problem hiding this comment.
Can you make sure this line is tested. Should be a super easy test to validate
src/ansys/motorcad/core/geometry.py
Outdated
|
|
||
| is_closed = get_entities_have_common_coordinate(entity_first, entity_last) | ||
| def is_nonintersecting(self): |
There was a problem hiding this comment.
I prefer is_self_intersecting() or just self_intersecting(). Also make this a property e.g.
There was a problem hiding this comment.
I think the implementation of this should be in EntityList
src/ansys/motorcad/core/geometry.py
Outdated
| """ | ||
| side_pairs = [] | ||
| # Create a set of all non-neighbor pairs of lines/arcs in the region | ||
| for entity in self.entities: |
There was a problem hiding this comment.
Nice implementation, a line sweep algorithm would be more efficient but more complicated to implement
src/ansys/motorcad/core/geometry.py
Outdated
| return is_closed | ||
| else: | ||
| return False | ||
| def is_anticlockwise(self): |
There was a problem hiding this comment.
This is an interesting implementation, would you be able to provide info on the implementation?
There was a problem hiding this comment.
The intuition is that if you're on the outermost edge of a closed shape, the drawing direction at that edge will also be the direction of the overall polygon. This implementation finds an outermost edge by looking for the lowest point, then determines the direction there using the ingoing and outgoing entities
aidanc-ansys
changed the title
Implements and fixes functions to check the geometric properties of EntityLists, as well as an EntityList constructor to generate an EntityList when given points.