update unify cycles #1413
update unify cycles #1413
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1413 +/- ##
==========================================
+ Coverage 61.57% 61.68% +0.10%
==========================================
Files 207 207
Lines 22281 22292 +11
==========================================
+ Hits 13720 13751 +31
+ Misses 8561 8541 -20 ☔ View full report in Codecov by Sentry. |
| """Unify the cycles of the mesh. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| root : str, optional |
src/compas/topology/orientation.py
Outdated
| @@ -166,6 +172,10 @@ def unify_cycles(vertices, faces, root=None): | |||
| The faces of the mesh defined as lists of vertex indices. | |||
| root : str, optional | |||
src/compas/topology/orientation.py
Outdated
| if nmax is not None: | ||
| if radius is not None: | ||
| return _face_adjacency(points, faces, nmax=nmax, radius=radius) | ||
| return _face_adjacency(points, faces, nmax=nmax) | ||
| if radius is not None: | ||
| return _face_adjacency(points, faces, radius=radius) |
There was a problem hiding this comment.
perhaps it would make sense to refactor _face_adjacency such that you can write here
if nmax or radius:
return _face_adjacency(points, faces, nmax=nmax, radius=radius)
|
btw, since the switch is no longer based on the number of faces, it should be easier to test using generated boundary cases... |
correct. do you have something in mind or should I leave the tests for now? |
|
was thinking that we could use the meshgrid with awkward proportions in x and y to generate inputs where radius and nmax provide very different results in different directions... |
As noted in #1411 there is a problem with face_adjacency on meshes with arbitrary-sized faces. I've now exposed the parameters
radiusandnmaxfromcompas.topology._face_adjacencytocompas.topology.face_adjacencyand further propagated them tounify_cyclesandMesh.unify_cycles.What type of change is this?
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.CHANGELOG.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed).invoke test).invoke lint).compas.datastructures.Mesh.