Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2609 +/- ##
==========================================
- Coverage 94.89% 91.62% -3.27%
==========================================
Files 173 173
Lines 14441 14513 +72
==========================================
- Hits 13704 13298 -406
- Misses 737 1215 +478 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
RobPasMue
left a comment
There was a problem hiding this comment.
It's looking good! Left some comments
|
|
||
| @property | ||
| @graphics_required | ||
| def visualization_polydata(self) -> "pv.PolyData": |
There was a problem hiding this comment.
Nice approach as well.
BTW... maybe it'd be a good idea to share the implementation with the Sketch bjects now that I think of it. We were directly using the PyVista Circle, Ellipse classes there - but if we have a common implementation it might be for the best. What do you think?
Also, the sketches currently show up as "solid" because the inner part of the ellipse and the circle get filled with a color. This implementation you have avoids that if I am not mistaken
There was a problem hiding this comment.
Avoiding the fill was the reason for using the discretized implementation - I figured it would be ok for visualization because ultra precision is not necessary. I thought the "no fill" was better for visualizing but lmk your thoughts!
There was a problem hiding this comment.
Fully agreed! I prefer this approach way more - that's what I was trying to suggest. To share this implementation with the sketch objects impacted (Circle, Ellipse). That way we avoid having "solid" sketches (which is really confusing!)
…ys-geometry into feat/plotting-curves
Description
Issue linked
Closes #2605
Checklist
feat: extrude circle to cylinder)