Fix hull failure with collinear circle centres - #2093
Conversation
A circle tangent to the hull without contributing to it enters as a zero-span arc, which makeCircle turns into a full circle, so the wire cannot close. Skip such arcs. Intermittent because convert_and_validate returns list(set(arcs)) and Arc is unhashable, so the traversal order follows id().
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2093 +/- ##
==========================================
+ Coverage 95.67% 95.74% +0.06%
==========================================
Files 30 30
Lines 9534 9540 +6
Branches 1421 1422 +1
==========================================
+ Hits 9122 9134 +12
+ Misses 255 251 -4
+ Partials 157 155 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I suppose adding |
Absolutely, I was aiming for a minimal, clean, non-intrusive patch. |
|
btw, the type check in |
convert_and_validate returns list(set(arcs)); without __hash__ the set was keyed on id(), so the order varied between runs. Keying on the arc definition makes it stable, and identical arcs now collapse instead of reaching arc_arc with a zero distance between centres. Both __eq__ compare the type first, otherwise == is not commutative between the two classes, and arc == None would raise instead of returning False. Strict rather than isinstance: a subclass adding fields would inherit a hash built from the base fields only. test_collinear permutes its input instead of repeating it - with a stable order the repeats would all take the same path.
2 participants
Sketch.hull()fails on three equal circles with collinear centres:The tangent line between the outer two circles also touches the middle one, so it
enters the hull spanning zero degrees.
finalize_hull()then builds a full circleinstead of a zero-length arc and the wire cannot close.
It only fails about two runs in three:
convert_and_validate()returnslist(set(arcs))andArcis unhashable, so the traversal order followsid().Hence the repeats in the test.
Same guard as in #1344, which has been open since June 2023 and bundles three
other changes. This is just that guard, plus a test - happy to close it if #1344
is revived instead.