-
Notifications
You must be signed in to change notification settings - Fork 543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CylinderSource
class
#4890
Add CylinderSource
class
#4890
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4890 +/- ##
=======================================
Coverage 95.98% 95.99%
=======================================
Files 131 131
Lines 21560 21607 +47
=======================================
+ Hits 20694 20741 +47
Misses 866 866 |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Approving this PR because tkoyama010 said so in here 😬
""" | ||
self.Update() | ||
output = wrap(self.GetOutput()) | ||
output.rotate_z(-90, inplace=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I wasn't able to review this timely. I have not tested this, but due to the rotation and translation in this method, could we get different result in these two cases for cyl_source = pv.CylinderSource()
, cyl_source.output.plot()
and pl.add_mesh(cyl_source)
. I think the latter will not have this rotation and translation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are always happy to review, no matter when. Please review whenever you like.
And very good point. This is a bug and I have created #4956 .
Overview
ConeSource
class #4860 . AddCylinderSource
class.translate
function fromgeometric_objects.py
togeometric_sources.py
.Details